public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Zeng, Star" <star.zeng@intel.com>
To: "Yao, Jiewen" <jiewen.yao@intel.com>,
	Laszlo Ersek <lersek@redhat.com>,
	"Duran, Leo" <leo.duran@amd.com>,
	"edk2-devel@ml01.01.org" <edk2-devel@ml01.01.org>
Cc: "Tian, Feng" <feng.tian@intel.com>,
	"Singh, Brijesh" <brijesh.singh@amd.com>,
	"Justen, Jordan L" <jordan.l.justen@intel.com>,
	"Zeng, Star" <star.zeng@intel.com>
Subject: Re: [PATCH] MdeModulePkg: Add dynamic PCD PcdPteMemoryEncryptionAddressOrMask
Date: Thu, 9 Feb 2017 05:56:18 +0000	[thread overview]
Message-ID: <0C09AFA07DD0434D9E2A0C6AEB0483103B825245@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C503A8EBB28@shsmsx102.ccr.corp.intel.com>

Stick to current comments and code, OvmfPkg X64 has bug? :)

PCD comments:
# It is assumed that 64-bit DxeCore is built in firmware if it is true; otherwise 32-bit DxeCore

Code pieces in S3ResumePei, S3SaveStateDxe, SmmLockBoxPeiLib, etc:
// Both BIOS and OS wants 64bit vector
if (FeaturePcdGet (PcdDxeIplSwitchToLongMode)) {
  return TRUE;
}

//
// BIOS supports 64bit waking vector.
//
if(FeaturePcdGet (PcdDxeIplSwitchToLongMode)) {
  returnTRUE;
}

if ((sizeof(UINTN) == sizeof(UINT32)) && (FeaturePcdGet (PcdDxeIplSwitchToLongMode)) ) {
  //
 // 32 PEI + 64 DXE
 //


According to above, the patch I drafted is wrong.

Thanks,
Star
From: Yao, Jiewen
Sent: Thursday, February 9, 2017 1:23 PM
To: Zeng, Star <star.zeng@intel.com>; Laszlo Ersek <lersek@redhat.com>; Duran, Leo <leo.duran@amd.com>; edk2-devel@ml01.01.org
Cc: Tian, Feng <feng.tian@intel.com>; Singh, Brijesh <brijesh.singh@amd.com>; Justen, Jordan L <jordan.l.justen@intel.com>
Subject: RE: [edk2] [PATCH] MdeModulePkg: Add dynamic PCD PcdPteMemoryEncryptionAddressOrMask

I suggest we evaluate *all* usages of PcdDxeIplSwitchToLongMode because this is an incompatible change.

# It is assumed that 64-bit DxeCore is built in firmware if it is true; otherwise 32-bit DxeCore

Thank you
Yao Jiewen

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

In fact, X64 DxeIplPeim does not refer PcdDxeIplSwitchToLongMode at all.

DxeIpl.inf:
[FeaturePcd.IA32]
  gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode      ## CONSUMES

As I remember, I did a draft patch below before for the discussion about how to determine *PEI* is 32BITs or 64BITs in title "[edk2] [PATCH v3 2/4] IntelFrameworkModulePkg: BdsDxe: only allocate below 4 GB if needed", I can't find the archive link any more.

---
 MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c | 5 +++++
 MdeModulePkg/MdeModulePkg.dec                  | 4 +++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c
index 6488880..348e084 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c
@@ -43,6 +43,11 @@ HandOffToDxeCore (
   EFI_PEI_VECTOR_HANDOFF_INFO_PPI *VectorHandoffInfoPpi;

   //
+  // It should be FALSE for both PEI and DXE are 64-bit.
+  //
+  ASSERT (PcdGetBool (PcdDxeIplSwitchToLongMode) == FALSE);
+
+  //
   // Get Vector Hand-off Info PPI and build Guided HOB
   //
   Status = PeiServicesLocatePpi (
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index af7bcab..4a73f7b 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -712,8 +712,10 @@
   ## Indicates if DxeIpl should switch to long mode to enter DXE phase.
   #  It is assumed that 64-bit DxeCore is built in firmware if it is true; otherwise 32-bit DxeCore
   #  is built in firmware.<BR><BR>
+  #  And it should be FALSE for both PEI and DXE are 64-bit.
   #   TRUE  - DxeIpl will load a 64-bit DxeCore and switch to long mode to hand over to DxeCore.<BR>
-  #   FALSE - DxeIpl will load a 32-bit DxeCore and perform stack switch to hand over to DxeCore.<BR>
+  #   FALSE - DxeIpl will load a 32-bit DxeCore and perform stack switch to hand over to DxeCore,<BR>
+  #           or both PEI and DXE are 64-bit.<BR>
   # @Prompt DxeIpl switch to long mode.
   gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode|TRUE|BOOLEAN|0x0001003b

--


Thanks,
Star
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com]
Sent: Thursday, February 9, 2017 3:48 AM
To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Duran, Leo <leo.duran@amd.com<mailto:leo.duran@amd.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org>
Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; Singh, Brijesh <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>>; Justen, Jordan L <jordan.l.justen@intel.com<mailto:jordan.l.justen@intel.com>>
Subject: Re: [edk2] [PATCH] MdeModulePkg: Add dynamic PCD PcdPteMemoryEncryptionAddressOrMask

On 02/08/17 19:20, Yao, Jiewen wrote:
> Got it.
>
>
>
> If the means of PcdDxeIplSwitchtoLongMode  is unclear, we may add more
> description to make it clear.
>
>
>
> If we believe "PcdDxeIplSwitchtoLongMode  == DXE is Long mode" as
> final conclusion, can we treat that as a bug and fix OVMF X64?

I don't know how to "fix" that. What is there to fix?

Flipping the PCD to TRUE in OVMF X64 would break OVMF X64, and likely OVMF Ia32X64 too. For two reasons:


(1) Toggling the PCD would alter the behavior of the DXE IPL PEIM and/or the DXE Core, and that behavior has never ever been tested with OVMF X64.

The PcdDxeIplSwitchToLongMode=FALSE setting in the X64 DSC dates back to:

  commit 8fa729a8b1401f01c6fd8ddbcab45e4a4904fa9a
  Author: lgao4 <lgao4@6f19259b-4bc3-4df7-8a09-765794883524>
  Date:   Mon Mar 15 01:40:59 2010 +0000

      Merge the same type PCD section.

      git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@10243 6f19259b-4bc3-4df7-8a09-765794883524

and before that, to:

  commit 49ba9447c92d6fca214476381107a180d08e59d1
  Author: jljusten <jljusten@6f19259b-4bc3-4df7-8a09-765794883524>
  Date:   Wed May 27 21:10:18 2009 +0000

      Add initial version of Open Virtual Machine Firmware (OVMF) platform.

      git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@8398 6f19259b-4bc3-4df7-8a09-765794883524

In other words, OVMF X64 has always worked like this, since its inception.


(2) PEI code under OvmfPkg itself uses this PCD to identify a 32-bit DXE phase.

    //
    // If DXE is 32-bit, then we're done; PciBusDxe will degrade 64-bit MMIO
    // resources to 32-bit anyway. See DegradeResource() in
    // "PciResourceSupport.c".
    //
  #ifdef MDE_CPU_IA32
    if (!FeaturePcdGet (PcdDxeIplSwitchToLongMode)) {
      return FirstNonAddress;
    }
  #endif

and

    //
    // If DXE is 32-bit, then just return the traditional 64 MB cap.
    //
  #ifdef MDE_CPU_IA32
    if (!FeaturePcdGet (PcdDxeIplSwitchToLongMode)) {
      return SIZE_64MB;
    }
  #endif

Thanks
Laszlo

>
>
>
> Thank you
>
> Yao Jiewen
>
>
>
>
>
> *From:*Laszlo Ersek [mailto:lersek@redhat.com]
> *Sent:* Wednesday, February 8, 2017 9:52 AM
> *To:* Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Duran, Leo
> <leo.duran@amd.com<mailto:leo.duran@amd.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>;
> edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org>
> *Cc:* Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; Singh, Brijesh
> <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>>; Justen, Jordan L <jordan.l.justen@intel.com<mailto:jordan.l.justen@intel.com>>
> *Subject:* Re: [edk2] [PATCH] MdeModulePkg: Add dynamic PCD
> PcdPteMemoryEncryptionAddressOrMask
>
>
>
> 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.<BR><BR>
>
> 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 <lersek@redhat.com
<mailto:lersek@redhat.com%0b>> <mailto:lersek@redhat.com>>; Duran, Leo <leo.duran@amd.com
<mailto:leo.duran@amd.com %0b>> <mailto:leo.duran@amd.com>>;
>> Zeng, Star <star.zeng@intel.com
<mailto:star.zeng@intel.com%0b>> <mailto:star.zeng@intel.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org>
> <mailto:edk2-devel@ml01.01.org>
>> *Cc:* Tian, Feng <feng.tian@intel.com <mailto:feng.tian@intel.com<mailto:feng.tian@intel.com %3cmailto:feng.tian@intel.com>>>;
>> Singh, Brijesh <brijesh.singh@amd.com <mailto:brijesh.singh@amd.com<mailto:brijesh.singh@amd.com %3cmailto:brijesh.singh@amd.com>>>
>> *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 <jiewen.yao@intel.com
<mailto:jiewen.yao@intel.com%0b>> <mailto:jiewen.yao@intel.com%0b>> <mailto:jiewen.yao@intel.com>>;
> Duran, Leo <leo.duran@amd.com <mailto:leo.duran@amd.com%0b<mailto:leo.duran@amd.com %3cmailto:leo.duran@amd.com%0b>>>
> <mailto:leo.duran@amd.com>>; Zeng, Star <star.zeng@intel.com
<mailto:star.zeng@intel.com %0b>> <mailto:star.zeng@intel.com%0b>> <mailto:star.zeng@intel.com>>;
> edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org> <mailto:edk2-devel@ml01.01.org>
>> <mailto:edk2-devel@ml01.01.org>
>> Cc: Tian, Feng <feng.tian@intel.com
<mailto:feng.tian@intel.com%0b>> <mailto:feng.tian@intel.com%0b>> <mailto:feng.tian@intel.com>>; Singh,
> Brijesh <brijesh.singh@amd.com <mailto:brijesh.singh@amd.com%0b<mailto:brijesh.singh@amd.com %3cmailto:brijesh.singh@amd.com%0b>>>
> <mailto:brijesh.singh@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.
>>
>> 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 <star.zeng@intel.com<mailto:star.zeng@intel.com
<mailto:star.zeng@intel.com%3cmailto:star.zeng@intel.com%0b>> <mailto:star.zeng@intel.com%3cmailto:star.zeng@intel.com%0b>>
> <mailto:star.zeng@intel.com%3cmailto:star.zeng@intel.com>>>;
> edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>
> <mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>
>> <mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>>
>>> *Cc:* Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com
<mailto:lersek@redhat.com%3cmailto:lersek@redhat.com%0b>> <mailto:lersek@redhat.com%3cmailto:lersek@redhat.com%0b>>
> <mailto:lersek@redhat.com%3cmailto:lersek@redhat.com>>>; Tian, Feng
>>> <feng.tian@intel.com<mailto:feng.tian@intel.com
<mailto:feng.tian@intel.com%3cmailto:feng.tian@intel.com%0b>> <mailto:feng.tian@intel.com%3cmailto:feng.tian@intel.com%0b>>
> <mailto:feng.tian@intel.com%3cmailto:feng.tian@intel.com>>>; Singh,
> Brijesh <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com
<mailto:brijesh.singh@amd.com%3cmailto:brijesh.singh@amd.com%0b>> <mailto:brijesh.singh@amd.com%3cmailto:brijesh.singh@amd.com%0b>>
> <mailto:brijesh.singh@amd.com%3cmailto:brijesh.singh@amd.com>>>; Yao,
>>> Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com
<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com%0b>> <mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com%0b>>
> <mailto:jiewen.yao@intel.com%3cmailto: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 %0b>>>>> <mailto:leo.duran@amd.com<mailto:leo.duran@amd.com
<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com %0b>>>>> %3cmailto:leo.duran@amd.com
> <mailto:leo.duran@amd.com
<mailto:leo.duran@amd.com %0b>> %3cmailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com
> %3cmailto:leo.duran@amd.com%0b>> <mailto:leo.duran@amd.com
<mailto:leo.duran@amd.com %0b>> %3cmailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com
> %3cmailto:leo.duran@amd.com>>>>;
> edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>
> <mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>
>> <mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>>
>>> <mailto:edk2-devel@ml01.01.org>
>>>> Cc: Laszlo Ersek <lersek@redhat.com
<mailto:lersek@redhat.com%0b>> <mailto:lersek@redhat.com%0b>> <mailto:lersek@redhat.com%0b
<mailto:lersek@redhat.com%0b %0b>> <mailto:lersek@redhat.com%0b%0b>>
> <mailto:lersek@redhat.com%0b%3cmailto:lersek@redhat.com%0b>>>
> <mailto:lersek@redhat.com>>; Tian, Feng <feng.tian@intel.com
<mailto:feng.tian@intel.com %0b>> <mailto:feng.tian@intel.com%0b>> <mailto:feng.tian@intel.com%0b
<mailto:feng.tian@intel.com%0b %0b>> <mailto:feng.tian@intel.com%0b%0b>>
> <mailto:feng.tian@intel.com%0b%3cmailto:feng.tian@intel.com%0b>>>
> <mailto:feng.tian@intel.com>>;
>>>> Singh, Brijesh <brijesh.singh@amd.com
<mailto:brijesh.singh@amd.com%0b>> <mailto:brijesh.singh@amd.com%0b>> <mailto:brijesh.singh@amd.com%0b
<mailto:brijesh.singh@amd.com%0b %0b>> <mailto:brijesh.singh@amd.com%0b%0b>>
> <mailto:brijesh.singh@amd.com%0b%3cmailto:brijesh.singh@amd.com%0b>>>
> <mailto:brijesh.singh@amd.com>>; Zeng, Star <star.zeng@intel.com
<mailto:star.zeng@intel.com %0b>> <mailto:star.zeng@intel.com%0b>> <mailto:star.zeng@intel.com%0b
<mailto:star.zeng@intel.com%0b %0b>> <mailto:star.zeng@intel.com%0b%0b>>
> <mailto:star.zeng@intel.com%0b%3cmailto:star.zeng@intel.com%0b>>>
> <mailto:star.zeng@intel.com>>;
>>>> Yao, Jiewen <jiewen.yao@intel.com
<mailto:jiewen.yao@intel.com %0b>>>>> <mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com
<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com %0b>>>>> %3cmailto:jiewen.yao@intel.com
> <mailto:jiewen.yao@intel.com
<mailto:jiewen.yao@intel.com %0b>> %3cmailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com
> %3cmailto:jiewen.yao@intel.com%0b>> <mailto:jiewen.yao@intel.com
<mailto:jiewen.yao@intel.com %0b>> %3cmailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com
> %3cmailto: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<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>
> <mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>
>> <mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>>
>> <mailto:edk2-devel@ml01.01.org>
>>>> Cc: Laszlo Ersek <lersek@redhat.com
<mailto:lersek@redhat.com%0b>> <mailto:lersek@redhat.com%0b>> <mailto:lersek@redhat.com%0b
<mailto:lersek@redhat.com%0b %0b>> <mailto:lersek@redhat.com%0b%0b>>
> <mailto:lersek@redhat.com%0b%3cmailto:lersek@redhat.com%0b>>>
> <mailto:lersek@redhat.com>>; Tian, Feng <feng.tian@intel.com
<mailto:feng.tian@intel.com %0b>> <mailto:feng.tian@intel.com%0b>> <mailto:feng.tian@intel.com%0b
<mailto:feng.tian@intel.com%0b %0b>> <mailto:feng.tian@intel.com%0b%0b>>
> <mailto:feng.tian@intel.com%0b%3cmailto:feng.tian@intel.com%0b>>>
> <mailto:feng.tian@intel.com>>;
>>>> Brijesh Singh <brijesh.singh@amd.com
<mailto:brijesh.singh@amd.com%0b>> <mailto:brijesh.singh@amd.com%0b>> <mailto:brijesh.singh@amd.com%0b
<mailto:brijesh.singh@amd.com%0b %0b>> <mailto:brijesh.singh@amd.com%0b%0b>>
> <mailto:brijesh.singh@amd.com%0b%3cmailto:brijesh.singh@amd.com%0b>>>
> <mailto:brijesh.singh@amd.com>>; Zeng, Star <star.zeng@intel.com
<mailto:star.zeng@intel.com %0b>> <mailto:star.zeng@intel.com%0b>> <mailto:star.zeng@intel.com%0b
<mailto:star.zeng@intel.com%0b %0b>> <mailto:star.zeng@intel.com%0b%0b>>
> <mailto:star.zeng@intel.com%0b%3cmailto:star.zeng@intel.com%0b>>>
> <mailto:star.zeng@intel.com>>;
>>>> Leo Duran <leo.duran@amd.com
<mailto:leo.duran@amd.com %0b>>>>> <mailto:leo.duran@amd.com<mailto:leo.duran@amd.com
<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com %0b>>>>> %3cmailto:leo.duran@amd.com
> <mailto:leo.duran@amd.com
<mailto:leo.duran@amd.com %0b>> %3cmailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com
> %3cmailto:leo.duran@amd.com%0b>> <mailto:leo.duran@amd.com
<mailto:leo.duran@amd.com %0b>> %3cmailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com
> %3cmailto:leo.duran@amd.com>>>>
>>>> Subject: [edk2] [PATCH] MdeModulePkg: Add dynamic PCD
>>>> PcdPteMemoryEncryptionAddressOrMask
>>>>
>>>> From: Brijesh Singh <brijesh.singh@amd.com
<mailto:brijesh.singh@amd.com %0b>>>>> <mailto:brijesh.singh@amd.com<mailto:brijesh.singh@amd.com
<mailto:brijesh.singh@amd.com%3cmailto:brijesh.singh@amd.com %0b>>>>> %3cmailto:brijesh.singh@amd.com
> <mailto:brijesh.singh@amd.com
<mailto:brijesh.singh@amd.com %0b>> %3cmailto:brijesh.singh@amd.com%3cmailto:brijesh.singh@amd.com
> %3cmailto:brijesh.singh@amd.com%0b>> <mailto:brijesh.singh@amd.com
<mailto:brijesh.singh@amd.com %0b>> %3cmailto:brijesh.singh@amd.com%3cmailto:brijesh.singh@amd.com
> %3cmailto: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 %0b>>>>> <mailto:feng.tian@intel.com<mailto:feng.tian@intel.com
<mailto:feng.tian@intel.com%3cmailto:feng.tian@intel.com %0b>>>>> %3cmailto:feng.tian@intel.com
> <mailto:feng.tian@intel.com
<mailto:feng.tian@intel.com %0b>> %3cmailto:feng.tian@intel.com%3cmailto:feng.tian@intel.com
> %3cmailto:feng.tian@intel.com%0b>> <mailto:feng.tian@intel.com
<mailto:feng.tian@intel.com %0b>> %3cmailto:feng.tian@intel.com%3cmailto:feng.tian@intel.com
> %3cmailto:feng.tian@intel.com>>>>
>>>> Cc: Star Zeng <star.zeng@intel.com
<mailto:star.zeng@intel.com %0b>>>>> <mailto:star.zeng@intel.com<mailto:star.zeng@intel.com
<mailto:star.zeng@intel.com%3cmailto:star.zeng@intel.com %0b>>>>> %3cmailto:star.zeng@intel.com
> <mailto:star.zeng@intel.com
<mailto:star.zeng@intel.com %0b>> %3cmailto:star.zeng@intel.com%3cmailto:star.zeng@intel.com
> %3cmailto:star.zeng@intel.com%0b>> <mailto:star.zeng@intel.com
<mailto:star.zeng@intel.com %0b>> %3cmailto:star.zeng@intel.com%3cmailto:star.zeng@intel.com
> %3cmailto:star.zeng@intel.com>>>>
>>>> Cc: Laszlo Ersek <lersek@redhat.com
<mailto:lersek@redhat.com %0b>>>>> <mailto:lersek@redhat.com<mailto:lersek@redhat.com
<mailto:lersek@redhat.com%3cmailto:lersek@redhat.com %0b>>>>> %3cmailto:lersek@redhat.com
> <mailto:lersek@redhat.com
<mailto:lersek@redhat.com %0b>> %3cmailto:lersek@redhat.com%3cmailto:lersek@redhat.com
> %3cmailto:lersek@redhat.com%0b>> <mailto:lersek@redhat.com
<mailto:lersek@redhat.com %0b>> %3cmailto:lersek@redhat.com%3cmailto:lersek@redhat.com
> %3cmailto:lersek@redhat.com>>>>
>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>> Signed-off-by: Leo Duran <leo.duran@amd.com
<mailto:leo.duran@amd.com %0b>>>>> <mailto:leo.duran@amd.com<mailto:leo.duran@amd.com
<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com %0b>>>>> %3cmailto:leo.duran@amd.com
> <mailto:leo.duran@amd.com
<mailto:leo.duran@amd.com %0b>> %3cmailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com
> %3cmailto:leo.duran@amd.com%0b>> <mailto:leo.duran@amd.com
<mailto:leo.duran@amd.com %0b>> %3cmailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com
> %3cmailto: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++,
>>>> 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<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>
> <mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>
>> <mailto:edk2-devel@lists.01.org%3cmailto: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<mailto:edk2-devel@lists.01.org>
> <mailto:edk2-devel@lists.01.org> <mailto:edk2-devel@lists.01.org>
>> https://lists.01.org/mailman/listinfo/edk2-devel
>>
>


  reply	other threads:[~2017-02-09  5:56 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-07 19:53 [PATCH] MdeModulePkg: Add dynamic PCD Leo Duran
2017-02-07 19:53 ` [PATCH] MdeModulePkg: Add dynamic PCD PcdPteMemoryEncryptionAddressOrMask Leo Duran
2017-02-08  2:27   ` Zeng, Star
2017-02-08 16:59     ` Duran, Leo
2017-02-08 17:05       ` Yao, Jiewen
2017-02-08 17:10         ` Laszlo Ersek
2017-02-08 17:17           ` Yao, Jiewen
2017-02-08 17:27             ` Yao, Jiewen
2017-02-08 17:51               ` Laszlo Ersek
2017-02-08 18:20                 ` Yao, Jiewen
2017-02-08 19:47                   ` Laszlo Ersek
2017-02-09  5:12                     ` Zeng, Star
2017-02-09  5:22                       ` Yao, Jiewen
2017-02-09  5:56                         ` Zeng, Star [this message]
2017-02-09  9:10                           ` Laszlo Ersek
2017-02-09  9:18                             ` Zeng, Star
2017-02-09  5:26                       ` Zeng, Star
2017-02-09  9:13                         ` Laszlo Ersek
2017-02-09  9:17                           ` Zeng, Star
2017-02-09  9:46                             ` Laszlo Ersek
2017-02-08 17:55             ` Duran, Leo
2017-02-08 17:28           ` Duran, Leo
2017-02-08 17:56             ` Laszlo Ersek
2017-02-08 18:13               ` Yao, Jiewen
2017-02-08 18:36                 ` Laszlo Ersek
2017-02-08 17:52         ` Duran, Leo
2017-02-08  8:38   ` Laszlo Ersek
2017-02-08 15:12     ` Duran, Leo
2017-02-08 15:19   ` Gao, Liming
2017-02-08 17:11     ` Duran, Leo
2017-02-08 17:29       ` Yao, Jiewen
2017-02-08 18:30         ` Duran, Leo
2017-02-08 18:33           ` Yao, Jiewen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0C09AFA07DD0434D9E2A0C6AEB0483103B825245@shsmsx102.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox