public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* Re: [PATCH] Discussion about PcdDxeIplSwitchToLongMode
@ 2017-02-09 15:11 Duran, Leo
  0 siblings, 0 replies; only message in thread
From: Duran, Leo @ 2017-02-09 15:11 UTC (permalink / raw)
  To: 'Laszlo Ersek', Zeng, Star, Yao, Jiewen,
	edk2-devel@ml01.01.org
  Cc: Tian, Feng, Singh, Brijesh, Justen, Jordan L

All,

I've changed the 'Subject' line to avoid confusion.

Thanks,
Leo.

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Thursday, February 09, 2017 3:46 AM
> To: Zeng, Star <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.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
> 
> On 02/09/17 10:17, Zeng, Star wrote:
> > EFI_HOB_CPU?
> > Is there discussion in PIWG for it?
> 
> None that I'm aware of.
> 
> Thanks
> Laszlo
> 
> >
> > Thanks,
> > Star
> > -----Original Message-----
> > From: Laszlo Ersek [mailto:lersek@redhat.com]
> > Sent: Thursday, February 9, 2017 5:13 PM
> > To: Zeng, Star <star.zeng@intel.com>; Yao, Jiewen
> > <jiewen.yao@intel.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
> >
> > On 02/09/17 06:26, Zeng, Star wrote:
> >> Correct typo in below email.
> >>
> >> "about how to determine DXE is 32BITs or 64BITs" should be "about how
> >> to determine PEI is 32BITs or 64BITs".
> >>
> >> At that time, we were discussing if the code needs to allocate <4G
> >> ACPI table for PEI phase at S3 resume.
> >
> > Indeed. Although OVMF X64 has a 64-bit PEI phase, that PEI phase can
> access only <4G RAM. IIRC one suggestion was to introduce a new HOB for
> this, so that PEI can advertise such a limitation to DXE.
> >
> > Thanks
> > Laszlo
> >
> >
> >>
> >> Thanks,
> >> Star
> >> -----Original Message-----
> >> From: Zeng, Star
> >> Sent: Thursday, February 9, 2017 1:12 PM
> >> To: Laszlo Ersek <lersek@redhat.com>; Yao, Jiewen
> >> <jiewen.yao@intel.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>; Zeng, Star <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 DXE 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|B
> OOLEAN
> >> |
> >> 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>; Duran, Leo
> >> <leo.duran@amd.com>; Zeng, Star <star.zeng@intel.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
> >>
> >> 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>; Duran, Leo
> >>> <leo.duran@amd.com>; Zeng, Star <star.zeng@intel.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
> >>>
> >>>
> >>>
> >>> 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>>; 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>>
> >>>> *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>>;
> >>> 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%0b>> <mailto:feng.tian@intel.com>>;
> >>> Singh, Brijesh <brijesh.singh@amd.com
> >>> <mailto: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>>>;
> >>> 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>>
> >>>>> *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>>>; 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>>>; 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>>>;
> >>> 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>>>
> >>>>> *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<mailto:leo.duran@amd.com
> >>>>>> %3cmailto:leo.duran@amd.com
> >>> <mailto:leo.duran@amd.com
> >>> %3cmailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com
> >>> %3cmailto:leo.duran@amd.com%0b>> <mailto:leo.duran@amd.com
> >>> %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>
> >>>>>> Cc: Laszlo Ersek <lersek@redhat.com
> >>> <mailto:lersek@redhat.com%0b>> <mailto:lersek@redhat.com%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%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%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%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<mailto:jiewen.yao@intel.com
> >>>>>> %3cmailto:jiewen.yao@intel.com
> >>> <mailto:jiewen.yao@intel.com
> >>> %3cmailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com
> >>> %3cmailto:jiewen.yao@intel.com%0b>> <mailto:jiewen.yao@intel.com
> >>> %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>
> >>>>>> Cc: Laszlo Ersek <lersek@redhat.com
> >>> <mailto:lersek@redhat.com%0b>> <mailto:lersek@redhat.com%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%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%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%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<mailto:leo.duran@amd.com
> >>>>>> %3cmailto:leo.duran@amd.com
> >>> <mailto:leo.duran@amd.com
> >>> %3cmailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com
> >>> %3cmailto:leo.duran@amd.com%0b>> <mailto:leo.duran@amd.com
> >>> %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<mailto:brijesh.singh@amd.com
> >>>>>> %3cmailto:brijesh.singh@amd.com
> >>> <mailto:brijesh.singh@amd.com
> >>> %3cmailto:brijesh.singh@amd.com%3cmailto:brijesh.singh@amd.com
> >>> %3cmailto:brijesh.singh@amd.com%0b>>
> <mailto:brijesh.singh@amd.com
> >>> %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<mailto:feng.tian@intel.com
> >>>>>> %3cmailto:feng.tian@intel.com
> >>> <mailto:feng.tian@intel.com
> >>> %3cmailto:feng.tian@intel.com%3cmailto:feng.tian@intel.com
> >>> %3cmailto:feng.tian@intel.com%0b>> <mailto:feng.tian@intel.com
> >>> %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<mailto:star.zeng@intel.com
> >>>>>> %3cmailto:star.zeng@intel.com
> >>> <mailto:star.zeng@intel.com
> >>> %3cmailto:star.zeng@intel.com%3cmailto:star.zeng@intel.com
> >>> %3cmailto:star.zeng@intel.com%0b>> <mailto:star.zeng@intel.com
> >>> %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<mailto:lersek@redhat.com
> >>>>>> %3cmailto:lersek@redhat.com
> >>> <mailto:lersek@redhat.com
> >>> %3cmailto:lersek@redhat.com%3cmailto:lersek@redhat.com
> >>> %3cmailto:lersek@redhat.com%0b>> <mailto:lersek@redhat.com
> >>> %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<mailto:leo.duran@amd.com
> >>>>>> %3cmailto:leo.duran@amd.com
> >>> <mailto:leo.duran@amd.com
> >>> %3cmailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com
> >>> %3cmailto:leo.duran@amd.com%0b>> <mailto:leo.duran@amd.com
> >>> %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>
> >>>>>> 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>
> >>>> https://lists.01.org/mailman/listinfo/edk2-devel
> >>>>
> >>>
> >>
> >



^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2017-02-09 15:11 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-09 15:11 [PATCH] Discussion about PcdDxeIplSwitchToLongMode Duran, Leo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox