public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 0/7] ArmPlatformPkg/PrePi: stop exposing internal code via HOBs
@ 2017-11-30 15:24 Ard Biesheuvel
  2017-11-30 15:24 ` [PATCH 1/7] EmbeddedPkg BeagleBoardPkg: move special HOB reuse libraries into platform Ard Biesheuvel
                   ` (6 more replies)
  0 siblings, 7 replies; 17+ messages in thread
From: Ard Biesheuvel @ 2017-11-30 15:24 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, Ard Biesheuvel, Udit Kumar, Meenakshi Aggarwal,
	Sakar Arora

This removes the code from PrePi and MemoryInitPeiLib that keeps the primary
FV around forever so that DXE phase code can reuse some of the code inside
the PrePi module.

Cc: Udit Kumar <udit.kumar@nxp.com>
Cc: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
Cc: Sakar Arora <Sakar.Arora@arm.com>


Ard Biesheuvel (6):
  EmbeddedPkg BeagleBoardPkg: move special HOB reuse libraries into
    platform
  BeagleBoardPkg: create private PrePi implementation
  BeagleBoardPkg: clone MemoryInitPeiLib
  ArmPlatformPkg/PrePi: don't expose PE/COFF and LZMA libraries via HOBs
  ArmPlatformPkg/PrePi; call all constructors by hand
  ArmPlatformPkg/PrePi: remove bogus IntelFrameworkModulePkg reference

Meenakshi Aggarwal (1):
  ArmPlatformPkg/MemoryInitPeiLib: don't reserve primary FV in memory

 ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c                                                   |  69 -------
 ArmPlatformPkg/PrePi/PeiMPCore.inf                                                                |   2 -
 ArmPlatformPkg/PrePi/PeiUniCore.inf                                                               |   2 -
 ArmPlatformPkg/PrePi/PrePi.c                                                                      |  24 +--
 BeagleBoardPkg/BeagleBoardPkg.dsc                                                                 |  14 +-
 BeagleBoardPkg/BeagleBoardPkg.fdf                                                                 |   2 +-
 {EmbeddedPkg => BeagleBoardPkg}/Library/DxeHobPeCoffLib/DxeHobPeCoff.c                            |   0
 {EmbeddedPkg => BeagleBoardPkg}/Library/DxeHobPeCoffLib/DxeHobPeCoffLib.inf                       |   0
 {EmbeddedPkg => BeagleBoardPkg}/Library/LzmaHobCustomDecompressLib/LzmaHobCustomDecompressLib.c   |   0
 {EmbeddedPkg => BeagleBoardPkg}/Library/LzmaHobCustomDecompressLib/LzmaHobCustomDecompressLib.inf |   0
 BeagleBoardPkg/Library/MemoryInitPeiLib/MemoryInitPeiLib.c                                        | 198 ++++++++++++++++++++
 BeagleBoardPkg/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf                                      |  64 +++++++
 BeagleBoardPkg/PrePi/Arm/ArchPrePi.c                                                              |  29 +++
 BeagleBoardPkg/PrePi/Arm/ModuleEntryPoint.S                                                       | 130 +++++++++++++
 BeagleBoardPkg/PrePi/Arm/ModuleEntryPoint.asm                                                     | 148 +++++++++++++++
 {ArmPlatformPkg => BeagleBoardPkg}/PrePi/LzmaDecompress.h                                         |   0
 BeagleBoardPkg/PrePi/MainUniCore.c                                                                |  39 ++++
 BeagleBoardPkg/PrePi/PeiUniCore.inf                                                               | 104 ++++++++++
 BeagleBoardPkg/PrePi/PrePi.c                                                                      | 198 ++++++++++++++++++++
 BeagleBoardPkg/PrePi/PrePi.h                                                                      |  90 +++++++++
 EmbeddedPkg/EmbeddedPkg.dsc                                                                       |   2 -
 21 files changed, 1012 insertions(+), 103 deletions(-)
 rename {EmbeddedPkg => BeagleBoardPkg}/Library/DxeHobPeCoffLib/DxeHobPeCoff.c (100%)
 rename {EmbeddedPkg => BeagleBoardPkg}/Library/DxeHobPeCoffLib/DxeHobPeCoffLib.inf (100%)
 rename {EmbeddedPkg => BeagleBoardPkg}/Library/LzmaHobCustomDecompressLib/LzmaHobCustomDecompressLib.c (100%)
 rename {EmbeddedPkg => BeagleBoardPkg}/Library/LzmaHobCustomDecompressLib/LzmaHobCustomDecompressLib.inf (100%)
 create mode 100644 BeagleBoardPkg/Library/MemoryInitPeiLib/MemoryInitPeiLib.c
 create mode 100644 BeagleBoardPkg/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf
 create mode 100644 BeagleBoardPkg/PrePi/Arm/ArchPrePi.c
 create mode 100644 BeagleBoardPkg/PrePi/Arm/ModuleEntryPoint.S
 create mode 100644 BeagleBoardPkg/PrePi/Arm/ModuleEntryPoint.asm
 rename {ArmPlatformPkg => BeagleBoardPkg}/PrePi/LzmaDecompress.h (100%)
 create mode 100644 BeagleBoardPkg/PrePi/MainUniCore.c
 create mode 100644 BeagleBoardPkg/PrePi/PeiUniCore.inf
 create mode 100644 BeagleBoardPkg/PrePi/PrePi.c
 create mode 100644 BeagleBoardPkg/PrePi/PrePi.h

-- 
2.11.0



^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/7] EmbeddedPkg BeagleBoardPkg: move special HOB reuse libraries into platform
  2017-11-30 15:24 [PATCH 0/7] ArmPlatformPkg/PrePi: stop exposing internal code via HOBs Ard Biesheuvel
@ 2017-11-30 15:24 ` Ard Biesheuvel
  2017-11-30 15:24 ` [PATCH 2/7] BeagleBoardPkg: create private PrePi implementation Ard Biesheuvel
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Ard Biesheuvel @ 2017-11-30 15:24 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, Ard Biesheuvel, Udit Kumar, Meenakshi Aggarwal,
	Sakar Arora

The BeagleBoard platform uses PeCoffLib and CustomDecompressLib
implementations that invoke the library code that resides in the PrePi
module via pointers exposed via special GUIDed HOBs. This is a nice hack,
but not necessarily something we want to carry in reference code.

So as a first step, move the libraries that expose this reused code into
BeagleBoardPkg, and remove it from EmbeddedPkg.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 BeagleBoardPkg/BeagleBoardPkg.dsc                                                                 | 6 +++---
 {EmbeddedPkg => BeagleBoardPkg}/Library/DxeHobPeCoffLib/DxeHobPeCoff.c                            | 0
 {EmbeddedPkg => BeagleBoardPkg}/Library/DxeHobPeCoffLib/DxeHobPeCoffLib.inf                       | 0
 {EmbeddedPkg => BeagleBoardPkg}/Library/LzmaHobCustomDecompressLib/LzmaHobCustomDecompressLib.c   | 0
 {EmbeddedPkg => BeagleBoardPkg}/Library/LzmaHobCustomDecompressLib/LzmaHobCustomDecompressLib.inf | 0
 EmbeddedPkg/EmbeddedPkg.dsc                                                                       | 2 --
 6 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/BeagleBoardPkg/BeagleBoardPkg.dsc b/BeagleBoardPkg/BeagleBoardPkg.dsc
index 2837ef3c0639..f74e58399989 100644
--- a/BeagleBoardPkg/BeagleBoardPkg.dsc
+++ b/BeagleBoardPkg/BeagleBoardPkg.dsc
@@ -172,7 +172,7 @@ [LibraryClasses.common.DXE_CORE]
   UefiDecompressLib|MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.inf
   DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
 #  PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
-  PeCoffLib|EmbeddedPkg/Library/DxeHobPeCoffLib/DxeHobPeCoffLib.inf
+  PeCoffLib|BeagleBoardPkg/Library/DxeHobPeCoffLib/DxeHobPeCoffLib.inf
 
   PerformanceLib|MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
 
@@ -203,7 +203,7 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
   ReportStatusCodeLib|IntelFrameworkModulePkg/Library/DxeReportStatusCodeLibFramework/DxeReportStatusCodeLib.inf
   CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
 #  PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
-  PeCoffLib|EmbeddedPkg/Library/DxeHobPeCoffLib/DxeHobPeCoffLib.inf
+  PeCoffLib|BeagleBoardPkg/Library/DxeHobPeCoffLib/DxeHobPeCoffLib.inf
 
 
 [LibraryClasses.ARM]
@@ -386,7 +386,7 @@ [Components.common]
     <LibraryClasses>
       PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
       NULL|MdeModulePkg/Library/DxeCrc32GuidedSectionExtractLib/DxeCrc32GuidedSectionExtractLib.inf
-      NULL|EmbeddedPkg/Library/LzmaHobCustomDecompressLib/LzmaHobCustomDecompressLib.inf
+      NULL|BeagleBoardPkg/Library/LzmaHobCustomDecompressLib/LzmaHobCustomDecompressLib.inf
   }
 
   ArmPkg/Drivers/CpuDxe/CpuDxe.inf
diff --git a/EmbeddedPkg/Library/DxeHobPeCoffLib/DxeHobPeCoff.c b/BeagleBoardPkg/Library/DxeHobPeCoffLib/DxeHobPeCoff.c
similarity index 100%
rename from EmbeddedPkg/Library/DxeHobPeCoffLib/DxeHobPeCoff.c
rename to BeagleBoardPkg/Library/DxeHobPeCoffLib/DxeHobPeCoff.c
diff --git a/EmbeddedPkg/Library/DxeHobPeCoffLib/DxeHobPeCoffLib.inf b/BeagleBoardPkg/Library/DxeHobPeCoffLib/DxeHobPeCoffLib.inf
similarity index 100%
rename from EmbeddedPkg/Library/DxeHobPeCoffLib/DxeHobPeCoffLib.inf
rename to BeagleBoardPkg/Library/DxeHobPeCoffLib/DxeHobPeCoffLib.inf
diff --git a/EmbeddedPkg/Library/LzmaHobCustomDecompressLib/LzmaHobCustomDecompressLib.c b/BeagleBoardPkg/Library/LzmaHobCustomDecompressLib/LzmaHobCustomDecompressLib.c
similarity index 100%
rename from EmbeddedPkg/Library/LzmaHobCustomDecompressLib/LzmaHobCustomDecompressLib.c
rename to BeagleBoardPkg/Library/LzmaHobCustomDecompressLib/LzmaHobCustomDecompressLib.c
diff --git a/EmbeddedPkg/Library/LzmaHobCustomDecompressLib/LzmaHobCustomDecompressLib.inf b/BeagleBoardPkg/Library/LzmaHobCustomDecompressLib/LzmaHobCustomDecompressLib.inf
similarity index 100%
rename from EmbeddedPkg/Library/LzmaHobCustomDecompressLib/LzmaHobCustomDecompressLib.inf
rename to BeagleBoardPkg/Library/LzmaHobCustomDecompressLib/LzmaHobCustomDecompressLib.inf
diff --git a/EmbeddedPkg/EmbeddedPkg.dsc b/EmbeddedPkg/EmbeddedPkg.dsc
index 073a1568844c..af965c2036cb 100644
--- a/EmbeddedPkg/EmbeddedPkg.dsc
+++ b/EmbeddedPkg/EmbeddedPkg.dsc
@@ -241,7 +241,6 @@ [Components.common]
   EmbeddedPkg/Library/PrePiLib/PrePiLib.inf
   EmbeddedPkg/Library/TemplateResetSystemLib/TemplateResetSystemLib.inf
   EmbeddedPkg/Library/TemplateRealTimeClockLib/TemplateRealTimeClockLib.inf
-  EmbeddedPkg/Library/LzmaHobCustomDecompressLib/LzmaHobCustomDecompressLib.inf
   EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf
   EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.inf
   EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
@@ -263,7 +262,6 @@ [Components.common]
 
   EmbeddedPkg/Library/AcpiLib/AcpiLib.inf
   EmbeddedPkg/Library/DebugAgentTimerLibNull/DebugAgentTimerLibNull.inf
-  EmbeddedPkg/Library/DxeHobPeCoffLib/DxeHobPeCoffLib.inf
   EmbeddedPkg/Library/FdtLib/FdtLib.inf
   EmbeddedPkg/Library/GdbDebugAgent/GdbDebugAgent.inf
   EmbeddedPkg/Library/PrePiHobLib/PrePiHobLib.inf
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/7] BeagleBoardPkg: create private PrePi implementation
  2017-11-30 15:24 [PATCH 0/7] ArmPlatformPkg/PrePi: stop exposing internal code via HOBs Ard Biesheuvel
  2017-11-30 15:24 ` [PATCH 1/7] EmbeddedPkg BeagleBoardPkg: move special HOB reuse libraries into platform Ard Biesheuvel
@ 2017-11-30 15:24 ` Ard Biesheuvel
  2017-11-30 15:24 ` [PATCH 3/7] BeagleBoardPkg: clone MemoryInitPeiLib Ard Biesheuvel
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Ard Biesheuvel @ 2017-11-30 15:24 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, Ard Biesheuvel, Udit Kumar, Meenakshi Aggarwal,
	Sakar Arora

Clone the PrePi implementation for BeagleBoardPkg, so we can start
removing some of the awkward optimizations that we'd rather not keep
in reference code. Note that we only clone the unicore ARM flavor,
which is all we need for BeagleBoard.

In the case of PrePi, it involves libraries included by the SEC
startup code that are exposed to DXE core via HOBs containing function
pointers, which forces us to keep the primary FV reserved in memory.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 BeagleBoardPkg/BeagleBoardPkg.dsc             |   6 +-
 BeagleBoardPkg/BeagleBoardPkg.fdf             |   2 +-
 BeagleBoardPkg/PrePi/Arm/ArchPrePi.c          |  29 +++
 BeagleBoardPkg/PrePi/Arm/ModuleEntryPoint.S   | 130 +++++++++++++
 BeagleBoardPkg/PrePi/Arm/ModuleEntryPoint.asm | 148 +++++++++++++++
 BeagleBoardPkg/PrePi/LzmaDecompress.h         | 103 ++++++++++
 BeagleBoardPkg/PrePi/MainUniCore.c            |  39 ++++
 BeagleBoardPkg/PrePi/PeiUniCore.inf           | 104 ++++++++++
 BeagleBoardPkg/PrePi/PrePi.c                  | 198 ++++++++++++++++++++
 BeagleBoardPkg/PrePi/PrePi.h                  |  90 +++++++++
 10 files changed, 846 insertions(+), 3 deletions(-)

diff --git a/BeagleBoardPkg/BeagleBoardPkg.dsc b/BeagleBoardPkg/BeagleBoardPkg.dsc
index f74e58399989..a1715593770a 100644
--- a/BeagleBoardPkg/BeagleBoardPkg.dsc
+++ b/BeagleBoardPkg/BeagleBoardPkg.dsc
@@ -142,7 +142,6 @@ [LibraryClasses.common.SEC]
   ReportStatusCodeLib|MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
   UefiDecompressLib|MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.inf
   ExtractGuidedSectionLib|EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.inf
-  LzmaDecompressLib|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
 
   PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
 
@@ -377,7 +376,10 @@ [Components.common]
   #
   # SEC
   #
-  ArmPlatformPkg/PrePi/PeiUniCore.inf
+  BeagleBoardPkg/PrePi/PeiUniCore.inf {
+    <LibraryClasses>
+      NULL|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
+  }
 
   #
   # DXE
diff --git a/BeagleBoardPkg/BeagleBoardPkg.fdf b/BeagleBoardPkg/BeagleBoardPkg.fdf
index 71249c7eb3d7..83d85fce4070 100644
--- a/BeagleBoardPkg/BeagleBoardPkg.fdf
+++ b/BeagleBoardPkg/BeagleBoardPkg.fdf
@@ -209,7 +209,7 @@ [FV.FVMAIN_COMPACT]
 READ_LOCK_CAP      = TRUE
 READ_LOCK_STATUS   = TRUE
 
-  INF ArmPlatformPkg/PrePi/PeiUniCore.inf
+  INF BeagleBoardPkg/PrePi/PeiUniCore.inf
 
   FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
     SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF PROCESSING_REQUIRED = TRUE {
diff --git a/BeagleBoardPkg/PrePi/Arm/ArchPrePi.c b/BeagleBoardPkg/PrePi/Arm/ArchPrePi.c
new file mode 100644
index 000000000000..075eb89e1b03
--- /dev/null
+++ b/BeagleBoardPkg/PrePi/Arm/ArchPrePi.c
@@ -0,0 +1,29 @@
+/** @file
+*
+*  Copyright (c) 2011 - 2013, ARM Limited. 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
+*  http://opensource.org/licenses/bsd-license.php
+*
+*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+*
+**/
+
+#include "PrePi.h"
+
+VOID
+ArchInitialize (
+  VOID
+  )
+{
+  // Enable program flow prediction, if supported.
+  ArmEnableBranchPrediction ();
+
+  if (FixedPcdGet32 (PcdVFPEnabled)) {
+    ArmEnableVFP ();
+  }
+}
+
diff --git a/BeagleBoardPkg/PrePi/Arm/ModuleEntryPoint.S b/BeagleBoardPkg/PrePi/Arm/ModuleEntryPoint.S
new file mode 100644
index 000000000000..212cab62d44b
--- /dev/null
+++ b/BeagleBoardPkg/PrePi/Arm/ModuleEntryPoint.S
@@ -0,0 +1,130 @@
+//
+//  Copyright (c) 2011-2015, ARM Limited. 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
+//  http://opensource.org/licenses/bsd-license.php
+//
+//  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+//  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+//
+//
+
+#include <AsmMacroIoLib.h>
+
+#include <Chipset/ArmV7.h>
+
+ASM_FUNC(_ModuleEntryPoint)
+  // Do early platform specific actions
+  bl    ASM_PFX(ArmPlatformPeiBootAction)
+
+  // Get ID of this CPU in Multicore system
+  bl    ASM_PFX(ArmReadMpidr)
+  // Keep a copy of the MpId register value
+  mov   r8, r0
+
+_SetSVCMode:
+  // Enter SVC mode, Disable FIQ and IRQ
+  mov     r1, #(CPSR_MODE_SVC | CPSR_IRQ | CPSR_FIQ)
+  msr     CPSR_c, r1
+
+// Check if we can install the stack at the top of the System Memory or if we need
+// to install the stacks at the bottom of the Firmware Device (case the FD is located
+// at the top of the DRAM)
+_SystemMemoryEndInit:
+  ADRL  (r1, mSystemMemoryEnd)
+  ldrd  r2, r3, [r1]
+  teq   r3, #0
+  moveq r1, r2
+  mvnne r1, #0
+
+_SetupStackPosition:
+  // r1 = SystemMemoryTop
+
+  // Calculate Top of the Firmware Device
+  MOV32 (r2, FixedPcdGet32(PcdFdBaseAddress))
+  MOV32 (r3, FixedPcdGet32(PcdFdSize) - 1)
+  add   r3, r3, r2      // r3 = FdTop = PcdFdBaseAddress + PcdFdSize
+
+  // UEFI Memory Size (stacks are allocated in this region)
+  MOV32 (r4, FixedPcdGet32(PcdSystemMemoryUefiRegionSize))
+
+  //
+  // Reserve the memory for the UEFI region (contain stacks on its top)
+  //
+
+  // Calculate how much space there is between the top of the Firmware and the Top of the System Memory
+  subs  r0, r1, r3      // r0 = SystemMemoryTop - FdTop
+  bmi   _SetupStack     // Jump if negative (FdTop > SystemMemoryTop). Case when the PrePi is in XIP memory outside of the DRAM
+  cmp   r0, r4
+  bge   _SetupStack
+
+  // Case the top of stacks is the FdBaseAddress
+  mov   r1, r2
+
+_SetupStack:
+  // r1 contains the top of the stack (and the UEFI Memory)
+
+  // Because the 'push' instruction is equivalent to 'stmdb' (decrement before), we need to increment
+  // one to the top of the stack. We check if incrementing one does not overflow (case of DRAM at the
+  // top of the memory space)
+  adds  r9, r1, #1
+  bcs   _SetupOverflowStack
+
+_SetupAlignedStack:
+  mov   r1, r9
+  b     _GetBaseUefiMemory
+
+_SetupOverflowStack:
+  // Case memory at the top of the address space. Ensure the top of the stack is EFI_PAGE_SIZE
+  // aligned (4KB)
+  MOV32 (r9, ~EFI_PAGE_MASK & 0xFFFFFFFF)
+  and   r1, r1, r9
+
+_GetBaseUefiMemory:
+  // Calculate the Base of the UEFI Memory
+  sub   r9, r1, r4
+
+_GetStackBase:
+  // r1 = The top of the Mpcore Stacks
+  // Stack for the primary core = PrimaryCoreStack
+  MOV32 (r2, FixedPcdGet32(PcdCPUCorePrimaryStackSize))
+  sub   r10, r1, r2
+
+  // Stack for the secondary core = Number of Cores - 1
+  MOV32 (r1, (FixedPcdGet32(PcdCoreCount) - 1) * FixedPcdGet32(PcdCPUCoreSecondaryStackSize))
+  sub   r10, r10, r1
+
+  // r10 = The base of the MpCore Stacks (primary stack & secondary stacks)
+  mov   r0, r10
+  mov   r1, r8
+  //ArmPlatformStackSet(StackBase, MpId, PrimaryStackSize, SecondaryStackSize)
+  MOV32 (r2, FixedPcdGet32(PcdCPUCorePrimaryStackSize))
+  MOV32 (r3, FixedPcdGet32(PcdCPUCoreSecondaryStackSize))
+  bl    ASM_PFX(ArmPlatformStackSet)
+
+  // Is it the Primary Core ?
+  mov   r0, r8
+  bl    ASM_PFX(ArmPlatformIsPrimaryCore)
+  cmp   r0, #1
+  bne   _PrepareArguments
+
+_PrepareArguments:
+  mov   r0, r8
+  mov   r1, r9
+  mov   r2, r10
+  mov   r3, sp
+
+  // Move sec startup address into a data register
+  // Ensure we're jumping to FV version of the code (not boot remapped alias)
+  ldr   r4, =ASM_PFX(CEntryPoint)
+
+  // Jump to PrePiCore C code
+  //    r0 = MpId
+  //    r1 = UefiMemoryBase
+  //    r2 = StacksBase
+  blx   r4
+
+_NeverReturn:
+  b _NeverReturn
diff --git a/BeagleBoardPkg/PrePi/Arm/ModuleEntryPoint.asm b/BeagleBoardPkg/PrePi/Arm/ModuleEntryPoint.asm
new file mode 100644
index 000000000000..80c2877cbdbf
--- /dev/null
+++ b/BeagleBoardPkg/PrePi/Arm/ModuleEntryPoint.asm
@@ -0,0 +1,148 @@
+//
+//  Copyright (c) 2011-2015, ARM Limited. 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
+//  http://opensource.org/licenses/bsd-license.php
+//
+//  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+//  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+//
+//
+
+#include <AutoGen.h>
+#include <Chipset/ArmV7.h>
+
+  INCLUDE AsmMacroIoLib.inc
+
+  IMPORT  CEntryPoint
+  IMPORT  ArmPlatformIsPrimaryCore
+  IMPORT  ArmReadMpidr
+  IMPORT  ArmPlatformPeiBootAction
+  IMPORT  ArmPlatformStackSet
+  IMPORT  mSystemMemoryEnd
+
+  EXPORT  _ModuleEntryPoint
+
+  PRESERVE8
+  AREA    PrePiCoreEntryPoint, CODE, READONLY
+
+StartupAddr        DCD      CEntryPoint
+
+_ModuleEntryPoint
+  // Do early platform specific actions
+  bl    ArmPlatformPeiBootAction
+
+  // Get ID of this CPU in Multicore system
+  bl    ArmReadMpidr
+  // Keep a copy of the MpId register value
+  mov   r8, r0
+
+_SetSVCMode
+  // Enter SVC mode, Disable FIQ and IRQ
+  mov     r1, #(CPSR_MODE_SVC :OR: CPSR_IRQ :OR: CPSR_FIQ)
+  msr     CPSR_c, r1
+
+// Check if we can install the stack at the top of the System Memory or if we need
+// to install the stacks at the bottom of the Firmware Device (case the FD is located
+// at the top of the DRAM)
+_SystemMemoryEndInit
+  adrll r1, mSystemMemoryEnd
+  ldrd  r2, r3, [r1]
+  teq   r3, #0
+  moveq r1, r2
+  mvnne r1, #0
+
+_SetupStackPosition
+  // r1 = SystemMemoryTop
+
+  // Calculate Top of the Firmware Device
+  mov32 r2, FixedPcdGet32(PcdFdBaseAddress)
+  mov32 r3, FixedPcdGet32(PcdFdSize)
+  sub   r3, r3, #1
+  add   r3, r3, r2      // r3 = FdTop = PcdFdBaseAddress + PcdFdSize
+
+  // UEFI Memory Size (stacks are allocated in this region)
+  mov32 r4, FixedPcdGet32(PcdSystemMemoryUefiRegionSize)
+
+  //
+  // Reserve the memory for the UEFI region (contain stacks on its top)
+  //
+
+  // Calculate how much space there is between the top of the Firmware and the Top of the System Memory
+  subs  r0, r1, r3      // r0 = SystemMemoryTop - FdTop
+  bmi   _SetupStack     // Jump if negative (FdTop > SystemMemoryTop). Case when the PrePi is in XIP memory outside of the DRAM
+  cmp   r0, r4
+  bge   _SetupStack
+
+  // Case the top of stacks is the FdBaseAddress
+  mov   r1, r2
+
+_SetupStack
+  // r1 contains the top of the stack (and the UEFI Memory)
+
+  // Because the 'push' instruction is equivalent to 'stmdb' (decrement before), we need to increment
+  // one to the top of the stack. We check if incrementing one does not overflow (case of DRAM at the
+  // top of the memory space)
+  adds  r9, r1, #1
+  bcs   _SetupOverflowStack
+
+_SetupAlignedStack
+  mov   r1, r9
+  b     _GetBaseUefiMemory
+
+_SetupOverflowStack
+  // Case memory at the top of the address space. Ensure the top of the stack is EFI_PAGE_SIZE
+  // aligned (4KB)
+  mov32 r9, EFI_PAGE_MASK
+  and   r9, r9, r1
+  sub   r1, r1, r9
+
+_GetBaseUefiMemory
+  // Calculate the Base of the UEFI Memory
+  sub   r9, r1, r4
+
+_GetStackBase
+  // r1 = The top of the Mpcore Stacks
+  // Stack for the primary core = PrimaryCoreStack
+  mov32 r2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)
+  sub   r10, r1, r2
+
+  // Stack for the secondary core = Number of Cores - 1
+  mov32 r1, (FixedPcdGet32(PcdCoreCount) - 1) * FixedPcdGet32(PcdCPUCoreSecondaryStackSize)
+  sub   r10, r10, r1
+
+  // r10 = The base of the MpCore Stacks (primary stack & secondary stacks)
+  mov   r0, r10
+  mov   r1, r8
+  //ArmPlatformStackSet(StackBase, MpId, PrimaryStackSize, SecondaryStackSize)
+  mov32 r2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)
+  mov32 r3, FixedPcdGet32(PcdCPUCoreSecondaryStackSize)
+  bl    ArmPlatformStackSet
+
+  // Is it the Primary Core ?
+  mov   r0, r8
+  bl    ArmPlatformIsPrimaryCore
+  cmp   r0, #1
+  bne   _PrepareArguments
+
+_PrepareArguments
+  mov   r0, r8
+  mov   r1, r9
+  mov   r2, r10
+
+  // Move sec startup address into a data register
+  // Ensure we're jumping to FV version of the code (not boot remapped alias)
+  ldr   r4, StartupAddr
+
+  // Jump to PrePiCore C code
+  //    r0 = MpId
+  //    r1 = UefiMemoryBase
+  //    r2 = StacksBase
+  blx   r4
+
+_NeverReturn
+  b _NeverReturn
+
+  END
diff --git a/BeagleBoardPkg/PrePi/LzmaDecompress.h b/BeagleBoardPkg/PrePi/LzmaDecompress.h
new file mode 100644
index 000000000000..a79ff343d231
--- /dev/null
+++ b/BeagleBoardPkg/PrePi/LzmaDecompress.h
@@ -0,0 +1,103 @@
+/** @file
+  LZMA Decompress Library header file
+
+  Copyright (c) 2006 - 2010, Intel Corporation. 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
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#ifndef __LZMA_DECOMPRESS_H___
+#define __LZMA_DECOMPRESS_H___
+
+/**
+  Examines a GUIDed section and returns the size of the decoded buffer and the
+  size of an scratch buffer required to actually decode the data in a GUIDed section.
+
+  Examines a GUIDed section specified by InputSection.
+  If GUID for InputSection does not match the GUID that this handler supports,
+  then RETURN_UNSUPPORTED is returned.
+  If the required information can not be retrieved from InputSection,
+  then RETURN_INVALID_PARAMETER is returned.
+  If the GUID of InputSection does match the GUID that this handler supports,
+  then the size required to hold the decoded buffer is returned in OututBufferSize,
+  the size of an optional scratch buffer is returned in ScratchSize, and the Attributes field
+  from EFI_GUID_DEFINED_SECTION header of InputSection is returned in SectionAttribute.
+
+  If InputSection is NULL, then ASSERT().
+  If OutputBufferSize is NULL, then ASSERT().
+  If ScratchBufferSize is NULL, then ASSERT().
+  If SectionAttribute is NULL, then ASSERT().
+
+
+  @param[in]  InputSection       A pointer to a GUIDed section of an FFS formatted file.
+  @param[out] OutputBufferSize   A pointer to the size, in bytes, of an output buffer required
+                                 if the buffer specified by InputSection were decoded.
+  @param[out] ScratchBufferSize  A pointer to the size, in bytes, required as scratch space
+                                 if the buffer specified by InputSection were decoded.
+  @param[out] SectionAttribute   A pointer to the attributes of the GUIDed section. See the Attributes
+                                 field of EFI_GUID_DEFINED_SECTION in the PI Specification.
+
+  @retval  RETURN_SUCCESS            The information about InputSection was returned.
+  @retval  RETURN_UNSUPPORTED        The section specified by InputSection does not match the GUID this handler supports.
+  @retval  RETURN_INVALID_PARAMETER  The information can not be retrieved from the section specified by InputSection.
+
+**/
+RETURN_STATUS
+EFIAPI
+LzmaGuidedSectionGetInfo (
+  IN  CONST VOID  *InputSection,
+  OUT UINT32      *OutputBufferSize,
+  OUT UINT32      *ScratchBufferSize,
+  OUT UINT16      *SectionAttribute
+  );
+
+/**
+  Decompress a LZAM compressed GUIDed section into a caller allocated output buffer.
+
+  Decodes the GUIDed section specified by InputSection.
+  If GUID for InputSection does not match the GUID that this handler supports, then RETURN_UNSUPPORTED is returned.
+  If the data in InputSection can not be decoded, then RETURN_INVALID_PARAMETER is returned.
+  If the GUID of InputSection does match the GUID that this handler supports, then InputSection
+  is decoded into the buffer specified by OutputBuffer and the authentication status of this
+  decode operation is returned in AuthenticationStatus.  If the decoded buffer is identical to the
+  data in InputSection, then OutputBuffer is set to point at the data in InputSection.  Otherwise,
+  the decoded data will be placed in caller allocated buffer specified by OutputBuffer.
+
+  If InputSection is NULL, then ASSERT().
+  If OutputBuffer is NULL, then ASSERT().
+  If ScratchBuffer is NULL and this decode operation requires a scratch buffer, then ASSERT().
+  If AuthenticationStatus is NULL, then ASSERT().
+
+
+  @param[in]  InputSection  A pointer to a GUIDed section of an FFS formatted file.
+  @param[out] OutputBuffer  A pointer to a buffer that contains the result of a decode operation.
+  @param[out] ScratchBuffer A caller allocated buffer that may be required by this function
+                            as a scratch buffer to perform the decode operation.
+  @param[out] AuthenticationStatus
+                            A pointer to the authentication status of the decoded output buffer.
+                            See the definition of authentication status in the EFI_PEI_GUIDED_SECTION_EXTRACTION_PPI
+                            section of the PI Specification. EFI_AUTH_STATUS_PLATFORM_OVERRIDE must
+                            never be set by this handler.
+
+  @retval  RETURN_SUCCESS            The buffer specified by InputSection was decoded.
+  @retval  RETURN_UNSUPPORTED        The section specified by InputSection does not match the GUID this handler supports.
+  @retval  RETURN_INVALID_PARAMETER  The section specified by InputSection can not be decoded.
+
+**/
+RETURN_STATUS
+EFIAPI
+LzmaGuidedSectionExtraction (
+  IN CONST  VOID    *InputSection,
+  OUT       VOID    **OutputBuffer,
+  OUT       VOID    *ScratchBuffer,        OPTIONAL
+  OUT       UINT32  *AuthenticationStatus
+  );
+
+#endif // __LZMADECOMPRESS_H__
+
diff --git a/BeagleBoardPkg/PrePi/MainUniCore.c b/BeagleBoardPkg/PrePi/MainUniCore.c
new file mode 100644
index 000000000000..56707362dcd1
--- /dev/null
+++ b/BeagleBoardPkg/PrePi/MainUniCore.c
@@ -0,0 +1,39 @@
+/** @file
+*
+*  Copyright (c) 2011, ARM Limited. All rights reserved.
+*  Copyright (c) 2017, Linaro, Ltd. 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
+*  http://opensource.org/licenses/bsd-license.php
+*
+*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+*
+**/
+
+#include "PrePi.h"
+
+VOID
+PrimaryMain (
+  IN  UINTN                     UefiMemoryBase,
+  IN  UINTN                     StacksBase,
+  IN  UINT64                    StartTimeStamp
+  )
+{
+  PrePiMain (UefiMemoryBase, StacksBase, StartTimeStamp);
+
+  // We must never return
+  ASSERT(FALSE);
+}
+
+VOID
+SecondaryMain (
+  IN  UINTN                     MpId
+  )
+{
+  // We must never get into this function on UniCore system
+  ASSERT(FALSE);
+}
+
diff --git a/BeagleBoardPkg/PrePi/PeiUniCore.inf b/BeagleBoardPkg/PrePi/PeiUniCore.inf
new file mode 100644
index 000000000000..3d72bc5b46e1
--- /dev/null
+++ b/BeagleBoardPkg/PrePi/PeiUniCore.inf
@@ -0,0 +1,104 @@
+#/** @file
+#
+#  (C) Copyright 2015 Hewlett-Packard Development Company, L.P.<BR>
+#  Copyright (c) 2011-2017, ARM Ltd. All rights reserved.<BR>
+#  Copyright (c) 2017, Linaro, Ltd. 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
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+#
+#**/
+
+[Defines]
+  INF_VERSION                    = 0x0001001A
+  BASE_NAME                      = BeagleBoardPrePiUniCore
+  FILE_GUID                      = 8a5dc3de-fe31-4ad9-9c93-dd73626932e7
+  MODULE_TYPE                    = SEC
+  VERSION_STRING                 = 1.0
+
+[Sources]
+  PrePi.c
+  MainUniCore.c
+
+[Sources.ARM]
+  Arm/ArchPrePi.c
+  Arm/ModuleEntryPoint.S   | GCC
+  Arm/ModuleEntryPoint.asm | RVCT
+
+[Packages]
+  ArmPkg/ArmPkg.dec
+  ArmPlatformPkg/ArmPlatformPkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  ArmLib
+  ArmPlatformLib
+  ArmPlatformStackLib
+  BaseLib
+  DebugLib
+  DebugAgentLib
+  ExtractGuidedSectionLib
+  HobLib
+  IoLib
+  MemoryAllocationLib
+  MemoryInitPeiLib
+  PeCoffGetEntryPointLib
+  PlatformPeiLib
+  PrePiHobListPointerLib
+  PrePiLib
+  SerialPortLib
+  TimerLib
+
+[Ppis]
+  gArmMpCoreInfoPpiGuid
+
+[Guids]
+  gArmMpCoreInfoGuid
+  gEfiFirmwarePerformanceGuid
+
+[FeaturePcd]
+  gEmbeddedTokenSpaceGuid.PcdPrePiProduceMemoryTypeInformationHob
+  gArmPlatformTokenSpaceGuid.PcdSendSgiToBringUpSecondaryCores
+
+[Pcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
+
+[FixedPcd]
+  gArmTokenSpaceGuid.PcdVFPEnabled
+
+  gArmTokenSpaceGuid.PcdFdBaseAddress
+  gArmTokenSpaceGuid.PcdFdSize
+
+  gArmTokenSpaceGuid.PcdFvBaseAddress
+  gArmTokenSpaceGuid.PcdFvSize
+
+  gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize
+  gArmPlatformTokenSpaceGuid.PcdCPUCoreSecondaryStackSize
+
+  gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize
+
+  gArmPlatformTokenSpaceGuid.PcdCoreCount
+
+  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
+  gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
+
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIMemoryNVS
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiReservedMemoryType
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesData
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesCode
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiBootServicesCode
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiBootServicesData
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderCode
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderData
+
+[Pcd]
+  gArmTokenSpaceGuid.PcdSystemMemoryBase
+  gArmTokenSpaceGuid.PcdSystemMemorySize
diff --git a/BeagleBoardPkg/PrePi/PrePi.c b/BeagleBoardPkg/PrePi/PrePi.c
new file mode 100644
index 000000000000..6c4f34692dfb
--- /dev/null
+++ b/BeagleBoardPkg/PrePi/PrePi.c
@@ -0,0 +1,198 @@
+/** @file
+*
+*  Copyright (c) 2011-2017, ARM Limited. All rights reserved.
+*  Copyright (c) 2017, Linaro, Ltd. 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
+*  http://opensource.org/licenses/bsd-license.php
+*
+*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+*
+**/
+
+#include <PiPei.h>
+
+#include <Library/DebugAgentLib.h>
+#include <Library/PrePiLib.h>
+#include <Library/PrintLib.h>
+#include <Library/PeCoffGetEntryPointLib.h>
+#include <Library/PrePiHobListPointerLib.h>
+#include <Library/TimerLib.h>
+#include <Library/PerformanceLib.h>
+
+#include <Ppi/GuidedSectionExtraction.h>
+#include <Ppi/ArmMpCoreInfo.h>
+#include <Ppi/SecPerformance.h>
+#include <Guid/LzmaDecompress.h>
+
+#include "PrePi.h"
+#include "LzmaDecompress.h"
+
+#define IS_XIP() (((UINT64)FixedPcdGet64 (PcdFdBaseAddress) > mSystemMemoryEnd) || \
+                  ((FixedPcdGet64 (PcdFdBaseAddress) + FixedPcdGet32 (PcdFdSize)) < FixedPcdGet64 (PcdSystemMemoryBase)))
+
+UINT64 mSystemMemoryEnd = FixedPcdGet64(PcdSystemMemoryBase) +
+                          FixedPcdGet64(PcdSystemMemorySize) - 1;
+
+EFI_STATUS
+EFIAPI
+ExtractGuidedSectionLibConstructor (
+  VOID
+  );
+
+EFI_STATUS
+EFIAPI
+LzmaDecompressLibConstructor (
+  VOID
+  );
+
+EFI_STATUS
+GetPlatformPpi (
+  IN  EFI_GUID  *PpiGuid,
+  OUT VOID      **Ppi
+  )
+{
+  UINTN                   PpiListSize;
+  UINTN                   PpiListCount;
+  EFI_PEI_PPI_DESCRIPTOR  *PpiList;
+  UINTN                   Index;
+
+  PpiListSize = 0;
+  ArmPlatformGetPlatformPpiList (&PpiListSize, &PpiList);
+  PpiListCount = PpiListSize / sizeof(EFI_PEI_PPI_DESCRIPTOR);
+  for (Index = 0; Index < PpiListCount; Index++, PpiList++) {
+    if (CompareGuid (PpiList->Guid, PpiGuid) == TRUE) {
+      *Ppi = PpiList->Ppi;
+      return EFI_SUCCESS;
+    }
+  }
+
+  return EFI_NOT_FOUND;
+}
+
+VOID
+PrePiMain (
+  IN  UINTN                     UefiMemoryBase,
+  IN  UINTN                     StacksBase,
+  IN  UINT64                    StartTimeStamp
+  )
+{
+  EFI_HOB_HANDOFF_INFO_TABLE*   HobList;
+  EFI_STATUS                    Status;
+  CHAR8                         Buffer[100];
+  UINTN                         CharCount;
+  UINTN                         StacksSize;
+  FIRMWARE_SEC_PERFORMANCE      Performance;
+
+  // If ensure the FD is either part of the System Memory or totally outside of the System Memory (XIP)
+  ASSERT (IS_XIP() ||
+          ((FixedPcdGet64 (PcdFdBaseAddress) >= FixedPcdGet64 (PcdSystemMemoryBase)) &&
+           ((UINT64)(FixedPcdGet64 (PcdFdBaseAddress) + FixedPcdGet32 (PcdFdSize)) <= (UINT64)mSystemMemoryEnd)));
+
+  // Initialize the architecture specific bits
+  ArchInitialize ();
+
+  // Initialize the Serial Port
+  SerialPortInitialize ();
+  CharCount = AsciiSPrint (Buffer,sizeof (Buffer),"UEFI firmware (version %s built at %a on %a)\n\r",
+    (CHAR16*)PcdGetPtr(PcdFirmwareVersionString), __TIME__, __DATE__);
+  SerialPortWrite ((UINT8 *) Buffer, CharCount);
+
+  // Initialize the Debug Agent for Source Level Debugging
+  InitializeDebugAgent (DEBUG_AGENT_INIT_POSTMEM_SEC, NULL, NULL);
+  SaveAndSetDebugTimerInterrupt (TRUE);
+
+  // Declare the PI/UEFI memory region
+  HobList = HobConstructor (
+    (VOID*)UefiMemoryBase,
+    FixedPcdGet32 (PcdSystemMemoryUefiRegionSize),
+    (VOID*)UefiMemoryBase,
+    (VOID*)StacksBase  // The top of the UEFI Memory is reserved for the stacks
+    );
+  PrePeiSetHobList (HobList);
+
+  // Initialize MMU and Memory HOBs (Resource Descriptor HOBs)
+  Status = MemoryPeim (UefiMemoryBase, FixedPcdGet32 (PcdSystemMemoryUefiRegionSize));
+  ASSERT_EFI_ERROR (Status);
+
+  StacksSize = PcdGet32 (PcdCPUCorePrimaryStackSize);
+  BuildStackHob (StacksBase, StacksSize);
+
+  //TODO: Call CpuPei as a library
+  BuildCpuHob (PcdGet8 (PcdPrePiCpuMemorySize), PcdGet8 (PcdPrePiCpuIoSize));
+
+  // Store timer value logged at the beginning of firmware image execution
+  Performance.ResetEnd = GetTimeInNanoSecond (StartTimeStamp);
+
+  // Build SEC Performance Data Hob
+  BuildGuidDataHob (&gEfiFirmwarePerformanceGuid, &Performance, sizeof (Performance));
+
+  // Set the Boot Mode
+  SetBootMode (ArmPlatformGetBootMode ());
+
+  // Initialize Platform HOBs (CpuHob and FvHob)
+  Status = PlatformPeim ();
+  ASSERT_EFI_ERROR (Status);
+
+  // Now, the HOB List has been initialized, we can register performance information
+  PERF_START (NULL, "PEI", NULL, StartTimeStamp);
+
+  // SEC phase needs to run library constructors by hand.
+  ExtractGuidedSectionLibConstructor ();
+  LzmaDecompressLibConstructor ();
+
+  // Build HOBs to pass up our version of stuff the DXE Core needs to save space
+  BuildPeCoffLoaderHob ();
+  BuildExtractSectionHob (
+    &gLzmaCustomDecompressGuid,
+    LzmaGuidedSectionGetInfo,
+    LzmaGuidedSectionExtraction
+    );
+
+  // Assume the FV that contains the SEC (our code) also contains a compressed FV.
+  Status = DecompressFirstFv ();
+  ASSERT_EFI_ERROR (Status);
+
+  // Load the DXE Core and transfer control to it
+  Status = LoadDxeCoreFromFv (NULL, 0);
+  ASSERT_EFI_ERROR (Status);
+}
+
+VOID
+CEntryPoint (
+  IN  UINTN                     MpId,
+  IN  UINTN                     UefiMemoryBase,
+  IN  UINTN                     StacksBase
+  )
+{
+  UINT64   StartTimeStamp;
+
+  // Initialize the platform specific controllers
+  ArmPlatformInitialize (MpId);
+
+  if (PerformanceMeasurementEnabled ()) {
+    // Initialize the Timer Library to setup the Timer HW controller
+    TimerConstructor ();
+    // We cannot call yet the PerformanceLib because the HOB List has not been initialized
+    StartTimeStamp = GetPerformanceCounter ();
+  } else {
+    StartTimeStamp = 0;
+  }
+
+  // Data Cache enabled on Primary core when MMU is enabled.
+  ArmDisableDataCache ();
+  // Invalidate Data cache
+  ArmInvalidateDataCache ();
+  // Invalidate instruction cache
+  ArmInvalidateInstructionCache ();
+  // Enable Instruction Caches on all cores.
+  ArmEnableInstructionCache ();
+
+  PrimaryMain (UefiMemoryBase, StacksBase, StartTimeStamp);
+
+  // DXE Core should always load and never return
+  ASSERT (FALSE);
+}
diff --git a/BeagleBoardPkg/PrePi/PrePi.h b/BeagleBoardPkg/PrePi/PrePi.h
new file mode 100644
index 000000000000..e7f58e59240c
--- /dev/null
+++ b/BeagleBoardPkg/PrePi/PrePi.h
@@ -0,0 +1,90 @@
+/** @file
+*
+*  Copyright (c) 2011-2015, ARM Limited. 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
+*  http://opensource.org/licenses/bsd-license.php
+*
+*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+*
+**/
+
+#ifndef _PREPI_H_
+#define _PREPI_H_
+
+#include <PiPei.h>
+
+#include <Library/PcdLib.h>
+#include <Library/ArmLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/IoLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/HobLib.h>
+#include <Library/SerialPortLib.h>
+#include <Library/ArmPlatformLib.h>
+
+#define SerialPrint(txt)  SerialPortWrite (txt, AsciiStrLen(txt)+1);
+
+extern UINT64 mSystemMemoryEnd;
+
+RETURN_STATUS
+EFIAPI
+TimerConstructor (
+  VOID
+  );
+
+VOID
+PrePiMain (
+  IN  UINTN                     UefiMemoryBase,
+  IN  UINTN                     StacksBase,
+  IN  UINT64                    StartTimeStamp
+  );
+
+EFI_STATUS
+EFIAPI
+MemoryPeim (
+  IN EFI_PHYSICAL_ADDRESS       UefiMemoryBase,
+  IN UINT64                     UefiMemorySize
+  );
+
+EFI_STATUS
+EFIAPI
+PlatformPeim (
+  VOID
+  );
+
+VOID
+PrimaryMain (
+  IN  UINTN                     UefiMemoryBase,
+  IN  UINTN                     StacksBase,
+  IN  UINT64                    StartTimeStamp
+  );
+
+VOID
+SecondaryMain (
+  IN  UINTN                     MpId
+  );
+
+// Either implemented by PrePiLib or by MemoryInitPei
+VOID
+BuildMemoryTypeInformationHob (
+  VOID
+  );
+
+EFI_STATUS
+GetPlatformPpi (
+  IN  EFI_GUID  *PpiGuid,
+  OUT VOID      **Ppi
+  );
+
+// Initialize the Architecture specific controllers
+VOID
+ArchInitialize (
+  VOID
+  );
+
+#endif /* _PREPI_H_ */
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/7] BeagleBoardPkg: clone MemoryInitPeiLib
  2017-11-30 15:24 [PATCH 0/7] ArmPlatformPkg/PrePi: stop exposing internal code via HOBs Ard Biesheuvel
  2017-11-30 15:24 ` [PATCH 1/7] EmbeddedPkg BeagleBoardPkg: move special HOB reuse libraries into platform Ard Biesheuvel
  2017-11-30 15:24 ` [PATCH 2/7] BeagleBoardPkg: create private PrePi implementation Ard Biesheuvel
@ 2017-11-30 15:24 ` Ard Biesheuvel
  2017-11-30 15:24 ` [PATCH 4/7] ArmPlatformPkg/PrePi: don't expose PE/COFF and LZMA libraries via HOBs Ard Biesheuvel
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Ard Biesheuvel @ 2017-11-30 15:24 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, Ard Biesheuvel, Udit Kumar, Meenakshi Aggarwal,
	Sakar Arora

The common MemoryInitPeiLib implementation preserves the primary FV
holding the PrePi module and the compressed secondary FV, and removes
the memory it occupies from the memory map, hiding it from the OS.

The only platform that actual requires this is BeagleBoardPkg, since
it exposes the PeCoff and LZMA libraries in PrePi to DXE core via
special HOBs. So let's give BeagleBoard its own MemoryInitPeiLib, so
that we can clean up the generic version.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 BeagleBoardPkg/BeagleBoardPkg.dsc                            |   2 +-
 BeagleBoardPkg/Library/MemoryInitPeiLib/MemoryInitPeiLib.c   | 198 ++++++++++++++++++++
 BeagleBoardPkg/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf |  64 +++++++
 3 files changed, 263 insertions(+), 1 deletion(-)

diff --git a/BeagleBoardPkg/BeagleBoardPkg.dsc b/BeagleBoardPkg/BeagleBoardPkg.dsc
index a1715593770a..5d87ee389124 100644
--- a/BeagleBoardPkg/BeagleBoardPkg.dsc
+++ b/BeagleBoardPkg/BeagleBoardPkg.dsc
@@ -150,7 +150,7 @@ [LibraryClasses.common.SEC]
   MemoryAllocationLib|EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf
   PerformanceLib|MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.inf
   PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
-  MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
+  MemoryInitPeiLib|BeagleBoardPkg/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf
 
   # 1/123 faster than Stm or Vstm version
   BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
diff --git a/BeagleBoardPkg/Library/MemoryInitPeiLib/MemoryInitPeiLib.c b/BeagleBoardPkg/Library/MemoryInitPeiLib/MemoryInitPeiLib.c
new file mode 100644
index 000000000000..2feb11f21d5d
--- /dev/null
+++ b/BeagleBoardPkg/Library/MemoryInitPeiLib/MemoryInitPeiLib.c
@@ -0,0 +1,198 @@
+/** @file
+*
+*  Copyright (c) 2011-2015, ARM Limited. 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
+*  http://opensource.org/licenses/bsd-license.php
+*
+*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+*
+**/
+
+#include <PiPei.h>
+
+#include <Library/ArmMmuLib.h>
+#include <Library/ArmPlatformLib.h>
+#include <Library/DebugLib.h>
+#include <Library/HobLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/PcdLib.h>
+
+VOID
+BuildMemoryTypeInformationHob (
+  VOID
+  );
+
+STATIC
+VOID
+InitMmu (
+  IN ARM_MEMORY_REGION_DESCRIPTOR  *MemoryTable
+  )
+{
+
+  VOID                          *TranslationTableBase;
+  UINTN                         TranslationTableSize;
+  RETURN_STATUS                 Status;
+
+  //Note: Because we called PeiServicesInstallPeiMemory() before to call InitMmu() the MMU Page Table resides in
+  //      DRAM (even at the top of DRAM as it is the first permanent memory allocation)
+  Status = ArmConfigureMmu (MemoryTable, &TranslationTableBase, &TranslationTableSize);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((EFI_D_ERROR, "Error: Failed to enable MMU\n"));
+  }
+}
+
+/*++
+
+Routine Description:
+
+
+
+Arguments:
+
+  FileHandle  - Handle of the file being invoked.
+  PeiServices - Describes the list of possible PEI Services.
+
+Returns:
+
+  Status -  EFI_SUCCESS if the boot mode could be set
+
+--*/
+EFI_STATUS
+EFIAPI
+MemoryPeim (
+  IN EFI_PHYSICAL_ADDRESS               UefiMemoryBase,
+  IN UINT64                             UefiMemorySize
+  )
+{
+  ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable;
+  EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;
+  UINT64                       ResourceLength;
+  EFI_PEI_HOB_POINTERS         NextHob;
+  EFI_PHYSICAL_ADDRESS         FdTop;
+  EFI_PHYSICAL_ADDRESS         SystemMemoryTop;
+  EFI_PHYSICAL_ADDRESS         ResourceTop;
+  BOOLEAN                      Found;
+
+  // Get Virtual Memory Map from the Platform Library
+  ArmPlatformGetVirtualMemoryMap (&MemoryTable);
+
+  // Ensure PcdSystemMemorySize has been set
+  ASSERT (PcdGet64 (PcdSystemMemorySize) != 0);
+
+  //
+  // Now, the permanent memory has been installed, we can call AllocatePages()
+  //
+  ResourceAttributes = (
+      EFI_RESOURCE_ATTRIBUTE_PRESENT |
+      EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
+      EFI_RESOURCE_ATTRIBUTE_WRITE_COMBINEABLE |
+      EFI_RESOURCE_ATTRIBUTE_WRITE_THROUGH_CACHEABLE |
+      EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE |
+      EFI_RESOURCE_ATTRIBUTE_TESTED
+  );
+
+  //
+  // Check if the resource for the main system memory has been declared
+  //
+  Found = FALSE;
+  NextHob.Raw = GetHobList ();
+  while ((NextHob.Raw = GetNextHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, NextHob.Raw)) != NULL) {
+    if ((NextHob.ResourceDescriptor->ResourceType == EFI_RESOURCE_SYSTEM_MEMORY) &&
+        (PcdGet64 (PcdSystemMemoryBase) >= NextHob.ResourceDescriptor->PhysicalStart) &&
+        (NextHob.ResourceDescriptor->PhysicalStart + NextHob.ResourceDescriptor->ResourceLength <= PcdGet64 (PcdSystemMemoryBase) + PcdGet64 (PcdSystemMemorySize)))
+    {
+      Found = TRUE;
+      break;
+    }
+    NextHob.Raw = GET_NEXT_HOB (NextHob);
+  }
+
+  if (!Found) {
+    // Reserved the memory space occupied by the firmware volume
+    BuildResourceDescriptorHob (
+        EFI_RESOURCE_SYSTEM_MEMORY,
+        ResourceAttributes,
+        PcdGet64 (PcdSystemMemoryBase),
+        PcdGet64 (PcdSystemMemorySize)
+    );
+  }
+
+  //
+  // Reserved the memory space occupied by the firmware volume
+  //
+
+  SystemMemoryTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdSystemMemoryBase) + (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdSystemMemorySize);
+  FdTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFdBaseAddress) + (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFdSize);
+
+  // EDK2 does not have the concept of boot firmware copied into DRAM. To avoid the DXE
+  // core to overwrite this area we must mark the region with the attribute non-present
+  if ((PcdGet64 (PcdFdBaseAddress) >= PcdGet64 (PcdSystemMemoryBase)) && (FdTop <= SystemMemoryTop)) {
+    Found = FALSE;
+
+    // Search for System Memory Hob that contains the firmware
+    NextHob.Raw = GetHobList ();
+    while ((NextHob.Raw = GetNextHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, NextHob.Raw)) != NULL) {
+      if ((NextHob.ResourceDescriptor->ResourceType == EFI_RESOURCE_SYSTEM_MEMORY) &&
+          (PcdGet64 (PcdFdBaseAddress) >= NextHob.ResourceDescriptor->PhysicalStart) &&
+          (FdTop <= NextHob.ResourceDescriptor->PhysicalStart + NextHob.ResourceDescriptor->ResourceLength))
+      {
+        ResourceAttributes = NextHob.ResourceDescriptor->ResourceAttribute;
+        ResourceLength = NextHob.ResourceDescriptor->ResourceLength;
+        ResourceTop = NextHob.ResourceDescriptor->PhysicalStart + ResourceLength;
+
+        if (PcdGet64 (PcdFdBaseAddress) == NextHob.ResourceDescriptor->PhysicalStart) {
+          if (SystemMemoryTop == FdTop) {
+            NextHob.ResourceDescriptor->ResourceAttribute = ResourceAttributes & ~EFI_RESOURCE_ATTRIBUTE_PRESENT;
+          } else {
+            // Create the System Memory HOB for the firmware with the non-present attribute
+            BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
+                                        ResourceAttributes & ~EFI_RESOURCE_ATTRIBUTE_PRESENT,
+                                        PcdGet64 (PcdFdBaseAddress),
+                                        PcdGet32 (PcdFdSize));
+
+            // Top of the FD is system memory available for UEFI
+            NextHob.ResourceDescriptor->PhysicalStart += PcdGet32(PcdFdSize);
+            NextHob.ResourceDescriptor->ResourceLength -= PcdGet32(PcdFdSize);
+          }
+        } else {
+          // Create the System Memory HOB for the firmware with the non-present attribute
+          BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
+                                      ResourceAttributes & ~EFI_RESOURCE_ATTRIBUTE_PRESENT,
+                                      PcdGet64 (PcdFdBaseAddress),
+                                      PcdGet32 (PcdFdSize));
+
+          // Update the HOB
+          NextHob.ResourceDescriptor->ResourceLength = PcdGet64 (PcdFdBaseAddress) - NextHob.ResourceDescriptor->PhysicalStart;
+
+          // If there is some memory available on the top of the FD then create a HOB
+          if (FdTop < NextHob.ResourceDescriptor->PhysicalStart + ResourceLength) {
+            // Create the System Memory HOB for the remaining region (top of the FD)
+            BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
+                                        ResourceAttributes,
+                                        FdTop,
+                                        ResourceTop - FdTop);
+          }
+        }
+        Found = TRUE;
+        break;
+      }
+      NextHob.Raw = GET_NEXT_HOB (NextHob);
+    }
+
+    ASSERT(Found);
+  }
+
+  // Build Memory Allocation Hob
+  InitMmu (MemoryTable);
+
+  if (FeaturePcdGet (PcdPrePiProduceMemoryTypeInformationHob)) {
+    // Optional feature that helps prevent EFI memory map fragmentation.
+    BuildMemoryTypeInformationHob ();
+  }
+
+  return EFI_SUCCESS;
+}
diff --git a/BeagleBoardPkg/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf b/BeagleBoardPkg/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf
new file mode 100644
index 000000000000..0e8c42f6c6bd
--- /dev/null
+++ b/BeagleBoardPkg/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf
@@ -0,0 +1,64 @@
+#/** @file
+#
+#  Copyright (c) 2011-2014, ARM Ltd. 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
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+#
+#**/
+
+[Defines]
+  INF_VERSION                    = 0x0001001A
+  BASE_NAME                      = BeagleBoardMemoryInitPeiLib
+  FILE_GUID                      = e489db0a-d847-4d67-910b-48a833f6fef5
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = MemoryInitPeiLib|SEC PEIM
+
+[Sources]
+  MemoryInitPeiLib.c
+
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+  ArmPkg/ArmPkg.dec
+  ArmPlatformPkg/ArmPlatformPkg.dec
+
+[LibraryClasses]
+  DebugLib
+  HobLib
+  ArmMmuLib
+  ArmPlatformLib
+
+[Guids]
+  gEfiMemoryTypeInformationGuid
+
+[FeaturePcd]
+  gEmbeddedTokenSpaceGuid.PcdPrePiProduceMemoryTypeInformationHob
+
+[FixedPcd]
+  gArmTokenSpaceGuid.PcdFdBaseAddress
+  gArmTokenSpaceGuid.PcdFdSize
+
+  gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize
+
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIMemoryNVS
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiReservedMemoryType
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesData
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesCode
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiBootServicesCode
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiBootServicesData
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderCode
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderData
+
+[Pcd]
+  gArmTokenSpaceGuid.PcdSystemMemoryBase
+  gArmTokenSpaceGuid.PcdSystemMemorySize
+
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 4/7] ArmPlatformPkg/PrePi: don't expose PE/COFF and LZMA libraries via HOBs
  2017-11-30 15:24 [PATCH 0/7] ArmPlatformPkg/PrePi: stop exposing internal code via HOBs Ard Biesheuvel
                   ` (2 preceding siblings ...)
  2017-11-30 15:24 ` [PATCH 3/7] BeagleBoardPkg: clone MemoryInitPeiLib Ard Biesheuvel
@ 2017-11-30 15:24 ` Ard Biesheuvel
  2017-11-30 15:24 ` [PATCH 5/7] ArmPlatformPkg/MemoryInitPeiLib: don't reserve primary FV in memory Ard Biesheuvel
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Ard Biesheuvel @ 2017-11-30 15:24 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, Ard Biesheuvel, Udit Kumar, Meenakshi Aggarwal,
	Sakar Arora

Avoid the need to preserve all memory exposed by PrePi indefinitely
by removing the 'feature' that exposes the PE/COFF and LZMA libraries
via special HOBs to other modules that may want to reuse the code.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPlatformPkg/PrePi/LzmaDecompress.h | 103 --------------------
 ArmPlatformPkg/PrePi/PeiMPCore.inf    |   1 -
 ArmPlatformPkg/PrePi/PeiUniCore.inf   |   1 -
 ArmPlatformPkg/PrePi/PrePi.c          |  11 ---
 4 files changed, 116 deletions(-)

diff --git a/ArmPlatformPkg/PrePi/LzmaDecompress.h b/ArmPlatformPkg/PrePi/LzmaDecompress.h
deleted file mode 100644
index a79ff343d231..000000000000
--- a/ArmPlatformPkg/PrePi/LzmaDecompress.h
+++ /dev/null
@@ -1,103 +0,0 @@
-/** @file
-  LZMA Decompress Library header file
-
-  Copyright (c) 2006 - 2010, Intel Corporation. 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
-  http://opensource.org/licenses/bsd-license.php
-
-  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-
-**/
-
-#ifndef __LZMA_DECOMPRESS_H___
-#define __LZMA_DECOMPRESS_H___
-
-/**
-  Examines a GUIDed section and returns the size of the decoded buffer and the
-  size of an scratch buffer required to actually decode the data in a GUIDed section.
-
-  Examines a GUIDed section specified by InputSection.
-  If GUID for InputSection does not match the GUID that this handler supports,
-  then RETURN_UNSUPPORTED is returned.
-  If the required information can not be retrieved from InputSection,
-  then RETURN_INVALID_PARAMETER is returned.
-  If the GUID of InputSection does match the GUID that this handler supports,
-  then the size required to hold the decoded buffer is returned in OututBufferSize,
-  the size of an optional scratch buffer is returned in ScratchSize, and the Attributes field
-  from EFI_GUID_DEFINED_SECTION header of InputSection is returned in SectionAttribute.
-
-  If InputSection is NULL, then ASSERT().
-  If OutputBufferSize is NULL, then ASSERT().
-  If ScratchBufferSize is NULL, then ASSERT().
-  If SectionAttribute is NULL, then ASSERT().
-
-
-  @param[in]  InputSection       A pointer to a GUIDed section of an FFS formatted file.
-  @param[out] OutputBufferSize   A pointer to the size, in bytes, of an output buffer required
-                                 if the buffer specified by InputSection were decoded.
-  @param[out] ScratchBufferSize  A pointer to the size, in bytes, required as scratch space
-                                 if the buffer specified by InputSection were decoded.
-  @param[out] SectionAttribute   A pointer to the attributes of the GUIDed section. See the Attributes
-                                 field of EFI_GUID_DEFINED_SECTION in the PI Specification.
-
-  @retval  RETURN_SUCCESS            The information about InputSection was returned.
-  @retval  RETURN_UNSUPPORTED        The section specified by InputSection does not match the GUID this handler supports.
-  @retval  RETURN_INVALID_PARAMETER  The information can not be retrieved from the section specified by InputSection.
-
-**/
-RETURN_STATUS
-EFIAPI
-LzmaGuidedSectionGetInfo (
-  IN  CONST VOID  *InputSection,
-  OUT UINT32      *OutputBufferSize,
-  OUT UINT32      *ScratchBufferSize,
-  OUT UINT16      *SectionAttribute
-  );
-
-/**
-  Decompress a LZAM compressed GUIDed section into a caller allocated output buffer.
-
-  Decodes the GUIDed section specified by InputSection.
-  If GUID for InputSection does not match the GUID that this handler supports, then RETURN_UNSUPPORTED is returned.
-  If the data in InputSection can not be decoded, then RETURN_INVALID_PARAMETER is returned.
-  If the GUID of InputSection does match the GUID that this handler supports, then InputSection
-  is decoded into the buffer specified by OutputBuffer and the authentication status of this
-  decode operation is returned in AuthenticationStatus.  If the decoded buffer is identical to the
-  data in InputSection, then OutputBuffer is set to point at the data in InputSection.  Otherwise,
-  the decoded data will be placed in caller allocated buffer specified by OutputBuffer.
-
-  If InputSection is NULL, then ASSERT().
-  If OutputBuffer is NULL, then ASSERT().
-  If ScratchBuffer is NULL and this decode operation requires a scratch buffer, then ASSERT().
-  If AuthenticationStatus is NULL, then ASSERT().
-
-
-  @param[in]  InputSection  A pointer to a GUIDed section of an FFS formatted file.
-  @param[out] OutputBuffer  A pointer to a buffer that contains the result of a decode operation.
-  @param[out] ScratchBuffer A caller allocated buffer that may be required by this function
-                            as a scratch buffer to perform the decode operation.
-  @param[out] AuthenticationStatus
-                            A pointer to the authentication status of the decoded output buffer.
-                            See the definition of authentication status in the EFI_PEI_GUIDED_SECTION_EXTRACTION_PPI
-                            section of the PI Specification. EFI_AUTH_STATUS_PLATFORM_OVERRIDE must
-                            never be set by this handler.
-
-  @retval  RETURN_SUCCESS            The buffer specified by InputSection was decoded.
-  @retval  RETURN_UNSUPPORTED        The section specified by InputSection does not match the GUID this handler supports.
-  @retval  RETURN_INVALID_PARAMETER  The section specified by InputSection can not be decoded.
-
-**/
-RETURN_STATUS
-EFIAPI
-LzmaGuidedSectionExtraction (
-  IN CONST  VOID    *InputSection,
-  OUT       VOID    **OutputBuffer,
-  OUT       VOID    *ScratchBuffer,        OPTIONAL
-  OUT       UINT32  *AuthenticationStatus
-  );
-
-#endif // __LZMADECOMPRESS_H__
-
diff --git a/ArmPlatformPkg/PrePi/PeiMPCore.inf b/ArmPlatformPkg/PrePi/PeiMPCore.inf
index 636049d4f44d..ccb5388e5516 100644
--- a/ArmPlatformPkg/PrePi/PeiMPCore.inf
+++ b/ArmPlatformPkg/PrePi/PeiMPCore.inf
@@ -52,7 +52,6 @@ [LibraryClasses]
   SerialPortLib
   ExtractGuidedSectionLib
   LzmaDecompressLib
-  PeCoffGetEntryPointLib
   DebugAgentLib
   PrePiLib
   ArmPlatformLib
diff --git a/ArmPlatformPkg/PrePi/PeiUniCore.inf b/ArmPlatformPkg/PrePi/PeiUniCore.inf
index f37ddec9d13d..2d376c30d400 100644
--- a/ArmPlatformPkg/PrePi/PeiUniCore.inf
+++ b/ArmPlatformPkg/PrePi/PeiUniCore.inf
@@ -51,7 +51,6 @@ [LibraryClasses]
   SerialPortLib
   ExtractGuidedSectionLib
   LzmaDecompressLib
-  PeCoffGetEntryPointLib
   DebugAgentLib
   PrePiLib
   ArmPlatformLib
diff --git a/ArmPlatformPkg/PrePi/PrePi.c b/ArmPlatformPkg/PrePi/PrePi.c
index e5016ea45359..a60cafdce45e 100644
--- a/ArmPlatformPkg/PrePi/PrePi.c
+++ b/ArmPlatformPkg/PrePi/PrePi.c
@@ -17,7 +17,6 @@
 #include <Library/DebugAgentLib.h>
 #include <Library/PrePiLib.h>
 #include <Library/PrintLib.h>
-#include <Library/PeCoffGetEntryPointLib.h>
 #include <Library/PrePiHobListPointerLib.h>
 #include <Library/TimerLib.h>
 #include <Library/PerformanceLib.h>
@@ -25,10 +24,8 @@
 #include <Ppi/GuidedSectionExtraction.h>
 #include <Ppi/ArmMpCoreInfo.h>
 #include <Ppi/SecPerformance.h>
-#include <Guid/LzmaDecompress.h>
 
 #include "PrePi.h"
-#include "LzmaDecompress.h"
 
 #define IS_XIP() (((UINT64)FixedPcdGet64 (PcdFdBaseAddress) > mSystemMemoryEnd) || \
                   ((FixedPcdGet64 (PcdFdBaseAddress) + FixedPcdGet32 (PcdFdSize)) < FixedPcdGet64 (PcdSystemMemoryBase)))
@@ -168,14 +165,6 @@ PrePiMain (
   ExtractGuidedSectionLibConstructor ();
   LzmaDecompressLibConstructor ();
 
-  // Build HOBs to pass up our version of stuff the DXE Core needs to save space
-  BuildPeCoffLoaderHob ();
-  BuildExtractSectionHob (
-    &gLzmaCustomDecompressGuid,
-    LzmaGuidedSectionGetInfo,
-    LzmaGuidedSectionExtraction
-    );
-
   // Assume the FV that contains the SEC (our code) also contains a compressed FV.
   Status = DecompressFirstFv ();
   ASSERT_EFI_ERROR (Status);
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 5/7] ArmPlatformPkg/MemoryInitPeiLib: don't reserve primary FV in memory
  2017-11-30 15:24 [PATCH 0/7] ArmPlatformPkg/PrePi: stop exposing internal code via HOBs Ard Biesheuvel
                   ` (3 preceding siblings ...)
  2017-11-30 15:24 ` [PATCH 4/7] ArmPlatformPkg/PrePi: don't expose PE/COFF and LZMA libraries via HOBs Ard Biesheuvel
@ 2017-11-30 15:24 ` Ard Biesheuvel
  2017-12-26 21:52   ` Vladimir Olovyannikov
  2017-11-30 15:24 ` [PATCH 6/7] ArmPlatformPkg/PrePi; call all constructors by hand Ard Biesheuvel
  2017-11-30 15:24 ` [PATCH 7/7] ArmPlatformPkg/PrePi: remove bogus IntelFrameworkModulePkg reference Ard Biesheuvel
  6 siblings, 1 reply; 17+ messages in thread
From: Ard Biesheuvel @ 2017-11-30 15:24 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, Ard Biesheuvel, Udit Kumar, Meenakshi Aggarwal,
	Sakar Arora

From: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>

Now that PrePi no longer exposes its internal code via special HOBs,
we can remove the special handling of the primary FV, which needed to
be reserved so that DXE core could call into the PE/COFF and LZMA
libraries in the PrePi module.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Udit Kumar <udit.kumar@nxp.com>
Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
[ardb: updated commit log]
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c | 69 --------------------
 1 file changed, 69 deletions(-)

diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
index 2feb11f21d5d..d03214b5df66 100644
--- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
+++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
@@ -70,11 +70,7 @@ MemoryPeim (
 {
   ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable;
   EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;
-  UINT64                       ResourceLength;
   EFI_PEI_HOB_POINTERS         NextHob;
-  EFI_PHYSICAL_ADDRESS         FdTop;
-  EFI_PHYSICAL_ADDRESS         SystemMemoryTop;
-  EFI_PHYSICAL_ADDRESS         ResourceTop;
   BOOLEAN                      Found;
 
   // Get Virtual Memory Map from the Platform Library
@@ -121,71 +117,6 @@ MemoryPeim (
     );
   }
 
-  //
-  // Reserved the memory space occupied by the firmware volume
-  //
-
-  SystemMemoryTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdSystemMemoryBase) + (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdSystemMemorySize);
-  FdTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFdBaseAddress) + (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFdSize);
-
-  // EDK2 does not have the concept of boot firmware copied into DRAM. To avoid the DXE
-  // core to overwrite this area we must mark the region with the attribute non-present
-  if ((PcdGet64 (PcdFdBaseAddress) >= PcdGet64 (PcdSystemMemoryBase)) && (FdTop <= SystemMemoryTop)) {
-    Found = FALSE;
-
-    // Search for System Memory Hob that contains the firmware
-    NextHob.Raw = GetHobList ();
-    while ((NextHob.Raw = GetNextHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, NextHob.Raw)) != NULL) {
-      if ((NextHob.ResourceDescriptor->ResourceType == EFI_RESOURCE_SYSTEM_MEMORY) &&
-          (PcdGet64 (PcdFdBaseAddress) >= NextHob.ResourceDescriptor->PhysicalStart) &&
-          (FdTop <= NextHob.ResourceDescriptor->PhysicalStart + NextHob.ResourceDescriptor->ResourceLength))
-      {
-        ResourceAttributes = NextHob.ResourceDescriptor->ResourceAttribute;
-        ResourceLength = NextHob.ResourceDescriptor->ResourceLength;
-        ResourceTop = NextHob.ResourceDescriptor->PhysicalStart + ResourceLength;
-
-        if (PcdGet64 (PcdFdBaseAddress) == NextHob.ResourceDescriptor->PhysicalStart) {
-          if (SystemMemoryTop == FdTop) {
-            NextHob.ResourceDescriptor->ResourceAttribute = ResourceAttributes & ~EFI_RESOURCE_ATTRIBUTE_PRESENT;
-          } else {
-            // Create the System Memory HOB for the firmware with the non-present attribute
-            BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
-                                        ResourceAttributes & ~EFI_RESOURCE_ATTRIBUTE_PRESENT,
-                                        PcdGet64 (PcdFdBaseAddress),
-                                        PcdGet32 (PcdFdSize));
-
-            // Top of the FD is system memory available for UEFI
-            NextHob.ResourceDescriptor->PhysicalStart += PcdGet32(PcdFdSize);
-            NextHob.ResourceDescriptor->ResourceLength -= PcdGet32(PcdFdSize);
-          }
-        } else {
-          // Create the System Memory HOB for the firmware with the non-present attribute
-          BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
-                                      ResourceAttributes & ~EFI_RESOURCE_ATTRIBUTE_PRESENT,
-                                      PcdGet64 (PcdFdBaseAddress),
-                                      PcdGet32 (PcdFdSize));
-
-          // Update the HOB
-          NextHob.ResourceDescriptor->ResourceLength = PcdGet64 (PcdFdBaseAddress) - NextHob.ResourceDescriptor->PhysicalStart;
-
-          // If there is some memory available on the top of the FD then create a HOB
-          if (FdTop < NextHob.ResourceDescriptor->PhysicalStart + ResourceLength) {
-            // Create the System Memory HOB for the remaining region (top of the FD)
-            BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
-                                        ResourceAttributes,
-                                        FdTop,
-                                        ResourceTop - FdTop);
-          }
-        }
-        Found = TRUE;
-        break;
-      }
-      NextHob.Raw = GET_NEXT_HOB (NextHob);
-    }
-
-    ASSERT(Found);
-  }
-
   // Build Memory Allocation Hob
   InitMmu (MemoryTable);
 
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 6/7] ArmPlatformPkg/PrePi; call all constructors by hand
  2017-11-30 15:24 [PATCH 0/7] ArmPlatformPkg/PrePi: stop exposing internal code via HOBs Ard Biesheuvel
                   ` (4 preceding siblings ...)
  2017-11-30 15:24 ` [PATCH 5/7] ArmPlatformPkg/MemoryInitPeiLib: don't reserve primary FV in memory Ard Biesheuvel
@ 2017-11-30 15:24 ` Ard Biesheuvel
  2017-11-30 16:31   ` Leif Lindholm
  2017-11-30 15:24 ` [PATCH 7/7] ArmPlatformPkg/PrePi: remove bogus IntelFrameworkModulePkg reference Ard Biesheuvel
  6 siblings, 1 reply; 17+ messages in thread
From: Ard Biesheuvel @ 2017-11-30 15:24 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, Ard Biesheuvel, Udit Kumar, Meenakshi Aggarwal,
	Sakar Arora

Call ProcessLibraryConstructorList () to invoke all library constructors
by hand rather than calling only some of them explicitly.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPlatformPkg/PrePi/PrePi.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/ArmPlatformPkg/PrePi/PrePi.c b/ArmPlatformPkg/PrePi/PrePi.c
index a60cafdce45e..76a53bc871d2 100644
--- a/ArmPlatformPkg/PrePi/PrePi.c
+++ b/ArmPlatformPkg/PrePi/PrePi.c
@@ -33,15 +33,9 @@
 UINT64 mSystemMemoryEnd = FixedPcdGet64(PcdSystemMemoryBase) +
                           FixedPcdGet64(PcdSystemMemorySize) - 1;
 
-EFI_STATUS
-EFIAPI
-ExtractGuidedSectionLibConstructor (
-  VOID
-  );
-
-EFI_STATUS
+VOID
 EFIAPI
-LzmaDecompressLibConstructor (
+ProcessLibraryConstructorList (
   VOID
   );
 
@@ -162,8 +156,7 @@ PrePiMain (
   PERF_START (NULL, "PEI", NULL, StartTimeStamp);
 
   // SEC phase needs to run library constructors by hand.
-  ExtractGuidedSectionLibConstructor ();
-  LzmaDecompressLibConstructor ();
+  ProcessLibraryConstructorList ();
 
   // Assume the FV that contains the SEC (our code) also contains a compressed FV.
   Status = DecompressFirstFv ();
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 7/7] ArmPlatformPkg/PrePi: remove bogus IntelFrameworkModulePkg reference
  2017-11-30 15:24 [PATCH 0/7] ArmPlatformPkg/PrePi: stop exposing internal code via HOBs Ard Biesheuvel
                   ` (5 preceding siblings ...)
  2017-11-30 15:24 ` [PATCH 6/7] ArmPlatformPkg/PrePi; call all constructors by hand Ard Biesheuvel
@ 2017-11-30 15:24 ` Ard Biesheuvel
  6 siblings, 0 replies; 17+ messages in thread
From: Ard Biesheuvel @ 2017-11-30 15:24 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, Ard Biesheuvel, Udit Kumar, Meenakshi Aggarwal,
	Sakar Arora

PrePi does not use anything from IntelFrameworkModulePkg so remove
the reference from its [Packages] sections.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPlatformPkg/PrePi/PeiMPCore.inf  | 1 -
 ArmPlatformPkg/PrePi/PeiUniCore.inf | 1 -
 2 files changed, 2 deletions(-)

diff --git a/ArmPlatformPkg/PrePi/PeiMPCore.inf b/ArmPlatformPkg/PrePi/PeiMPCore.inf
index ccb5388e5516..242b03175536 100644
--- a/ArmPlatformPkg/PrePi/PeiMPCore.inf
+++ b/ArmPlatformPkg/PrePi/PeiMPCore.inf
@@ -39,7 +39,6 @@ [Packages]
   EmbeddedPkg/EmbeddedPkg.dec
   ArmPkg/ArmPkg.dec
   ArmPlatformPkg/ArmPlatformPkg.dec
-  IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec
 
 [LibraryClasses]
   BaseLib
diff --git a/ArmPlatformPkg/PrePi/PeiUniCore.inf b/ArmPlatformPkg/PrePi/PeiUniCore.inf
index 2d376c30d400..a45cdef4ed91 100644
--- a/ArmPlatformPkg/PrePi/PeiUniCore.inf
+++ b/ArmPlatformPkg/PrePi/PeiUniCore.inf
@@ -39,7 +39,6 @@ [Packages]
   EmbeddedPkg/EmbeddedPkg.dec
   ArmPkg/ArmPkg.dec
   ArmPlatformPkg/ArmPlatformPkg.dec
-  IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec
 
 [LibraryClasses]
   BaseLib
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 6/7] ArmPlatformPkg/PrePi; call all constructors by hand
  2017-11-30 15:24 ` [PATCH 6/7] ArmPlatformPkg/PrePi; call all constructors by hand Ard Biesheuvel
@ 2017-11-30 16:31   ` Leif Lindholm
  2017-11-30 16:35     ` Ard Biesheuvel
  0 siblings, 1 reply; 17+ messages in thread
From: Leif Lindholm @ 2017-11-30 16:31 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel, Udit Kumar, Meenakshi Aggarwal, Sakar Arora

On Thu, Nov 30, 2017 at 03:24:52PM +0000, Ard Biesheuvel wrote:
> Call ProcessLibraryConstructorList () to invoke all library constructors
> by hand rather than calling only some of them explicitly.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmPlatformPkg/PrePi/PrePi.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/ArmPlatformPkg/PrePi/PrePi.c b/ArmPlatformPkg/PrePi/PrePi.c
> index a60cafdce45e..76a53bc871d2 100644
> --- a/ArmPlatformPkg/PrePi/PrePi.c
> +++ b/ArmPlatformPkg/PrePi/PrePi.c
> @@ -33,15 +33,9 @@
>  UINT64 mSystemMemoryEnd = FixedPcdGet64(PcdSystemMemoryBase) +
>                            FixedPcdGet64(PcdSystemMemorySize) - 1;
>  
> -EFI_STATUS
> -EFIAPI
> -ExtractGuidedSectionLibConstructor (
> -  VOID
> -  );
> -
> -EFI_STATUS
> +VOID
>  EFIAPI
> -LzmaDecompressLibConstructor (
> +ProcessLibraryConstructorList (
>    VOID
>    );

Hmm ...
Do we need this local declaration?
Is there some header we could use?
If not - could we move it out to PrePi.h?

> @@ -162,8 +156,7 @@ PrePiMain (
>    PERF_START (NULL, "PEI", NULL, StartTimeStamp);
>  
>    // SEC phase needs to run library constructors by hand.
> -  ExtractGuidedSectionLibConstructor ();
> -  LzmaDecompressLibConstructor ();
> +  ProcessLibraryConstructorList ();
>  
>    // Assume the FV that contains the SEC (our code) also contains a compressed FV.
>    Status = DecompressFirstFv ();
> -- 
> 2.11.0
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 6/7] ArmPlatformPkg/PrePi; call all constructors by hand
  2017-11-30 16:31   ` Leif Lindholm
@ 2017-11-30 16:35     ` Ard Biesheuvel
  2017-11-30 16:45       ` Leif Lindholm
  0 siblings, 1 reply; 17+ messages in thread
From: Ard Biesheuvel @ 2017-11-30 16:35 UTC (permalink / raw)
  To: Leif Lindholm
  Cc: edk2-devel@lists.01.org, Udit Kumar, Meenakshi Aggarwal,
	Sakar Arora

On 30 November 2017 at 16:31, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Thu, Nov 30, 2017 at 03:24:52PM +0000, Ard Biesheuvel wrote:
>> Call ProcessLibraryConstructorList () to invoke all library constructors
>> by hand rather than calling only some of them explicitly.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  ArmPlatformPkg/PrePi/PrePi.c | 13 +++----------
>>  1 file changed, 3 insertions(+), 10 deletions(-)
>>
>> diff --git a/ArmPlatformPkg/PrePi/PrePi.c b/ArmPlatformPkg/PrePi/PrePi.c
>> index a60cafdce45e..76a53bc871d2 100644
>> --- a/ArmPlatformPkg/PrePi/PrePi.c
>> +++ b/ArmPlatformPkg/PrePi/PrePi.c
>> @@ -33,15 +33,9 @@
>>  UINT64 mSystemMemoryEnd = FixedPcdGet64(PcdSystemMemoryBase) +
>>                            FixedPcdGet64(PcdSystemMemorySize) - 1;
>>
>> -EFI_STATUS
>> -EFIAPI
>> -ExtractGuidedSectionLibConstructor (
>> -  VOID
>> -  );
>> -
>> -EFI_STATUS
>> +VOID
>>  EFIAPI
>> -LzmaDecompressLibConstructor (
>> +ProcessLibraryConstructorList (
>>    VOID
>>    );
>
> Hmm ...
> Do we need this local declaration?
> Is there some header we could use?

That doesn't appear to be the case, no

> If not - could we move it out to PrePi.h?
>

Yes, that works for me.


>> @@ -162,8 +156,7 @@ PrePiMain (
>>    PERF_START (NULL, "PEI", NULL, StartTimeStamp);
>>
>>    // SEC phase needs to run library constructors by hand.
>> -  ExtractGuidedSectionLibConstructor ();
>> -  LzmaDecompressLibConstructor ();
>> +  ProcessLibraryConstructorList ();
>>
>>    // Assume the FV that contains the SEC (our code) also contains a compressed FV.
>>    Status = DecompressFirstFv ();
>> --
>> 2.11.0
>>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 6/7] ArmPlatformPkg/PrePi; call all constructors by hand
  2017-11-30 16:35     ` Ard Biesheuvel
@ 2017-11-30 16:45       ` Leif Lindholm
  2017-11-30 17:12         ` Ard Biesheuvel
  0 siblings, 1 reply; 17+ messages in thread
From: Leif Lindholm @ 2017-11-30 16:45 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel@lists.01.org

On Thu, Nov 30, 2017 at 04:35:41PM +0000, Ard Biesheuvel wrote:
> On 30 November 2017 at 16:31, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > On Thu, Nov 30, 2017 at 03:24:52PM +0000, Ard Biesheuvel wrote:
> >> Call ProcessLibraryConstructorList () to invoke all library constructors
> >> by hand rather than calling only some of them explicitly.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> ---
> >>  ArmPlatformPkg/PrePi/PrePi.c | 13 +++----------
> >>  1 file changed, 3 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/ArmPlatformPkg/PrePi/PrePi.c b/ArmPlatformPkg/PrePi/PrePi.c
> >> index a60cafdce45e..76a53bc871d2 100644
> >> --- a/ArmPlatformPkg/PrePi/PrePi.c
> >> +++ b/ArmPlatformPkg/PrePi/PrePi.c
> >> @@ -33,15 +33,9 @@
> >>  UINT64 mSystemMemoryEnd = FixedPcdGet64(PcdSystemMemoryBase) +
> >>                            FixedPcdGet64(PcdSystemMemorySize) - 1;
> >>
> >> -EFI_STATUS
> >> -EFIAPI
> >> -ExtractGuidedSectionLibConstructor (
> >> -  VOID
> >> -  );
> >> -
> >> -EFI_STATUS
> >> +VOID
> >>  EFIAPI
> >> -LzmaDecompressLibConstructor (
> >> +ProcessLibraryConstructorList (
> >>    VOID
> >>    );
> >
> > Hmm ...
> > Do we need this local declaration?
> > Is there some header we could use?
> 
> That doesn't appear to be the case, no

Seems like a future opportunity.

> > If not - could we move it out to PrePi.h?
> 
> Yes, that works for me.

OK, if you fold that in, for the series:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> >> @@ -162,8 +156,7 @@ PrePiMain (
> >>    PERF_START (NULL, "PEI", NULL, StartTimeStamp);
> >>
> >>    // SEC phase needs to run library constructors by hand.
> >> -  ExtractGuidedSectionLibConstructor ();
> >> -  LzmaDecompressLibConstructor ();
> >> +  ProcessLibraryConstructorList ();
> >>
> >>    // Assume the FV that contains the SEC (our code) also contains a compressed FV.
> >>    Status = DecompressFirstFv ();
> >> --
> >> 2.11.0
> >>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 6/7] ArmPlatformPkg/PrePi; call all constructors by hand
  2017-11-30 16:45       ` Leif Lindholm
@ 2017-11-30 17:12         ` Ard Biesheuvel
  0 siblings, 0 replies; 17+ messages in thread
From: Ard Biesheuvel @ 2017-11-30 17:12 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: edk2-devel@lists.01.org

On 30 November 2017 at 16:45, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Thu, Nov 30, 2017 at 04:35:41PM +0000, Ard Biesheuvel wrote:
>> On 30 November 2017 at 16:31, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>> > On Thu, Nov 30, 2017 at 03:24:52PM +0000, Ard Biesheuvel wrote:
>> >> Call ProcessLibraryConstructorList () to invoke all library constructors
>> >> by hand rather than calling only some of them explicitly.
>> >>
>> >> Contributed-under: TianoCore Contribution Agreement 1.1
>> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> >> ---
>> >>  ArmPlatformPkg/PrePi/PrePi.c | 13 +++----------
>> >>  1 file changed, 3 insertions(+), 10 deletions(-)
>> >>
>> >> diff --git a/ArmPlatformPkg/PrePi/PrePi.c b/ArmPlatformPkg/PrePi/PrePi.c
>> >> index a60cafdce45e..76a53bc871d2 100644
>> >> --- a/ArmPlatformPkg/PrePi/PrePi.c
>> >> +++ b/ArmPlatformPkg/PrePi/PrePi.c
>> >> @@ -33,15 +33,9 @@
>> >>  UINT64 mSystemMemoryEnd = FixedPcdGet64(PcdSystemMemoryBase) +
>> >>                            FixedPcdGet64(PcdSystemMemorySize) - 1;
>> >>
>> >> -EFI_STATUS
>> >> -EFIAPI
>> >> -ExtractGuidedSectionLibConstructor (
>> >> -  VOID
>> >> -  );
>> >> -
>> >> -EFI_STATUS
>> >> +VOID
>> >>  EFIAPI
>> >> -LzmaDecompressLibConstructor (
>> >> +ProcessLibraryConstructorList (
>> >>    VOID
>> >>    );
>> >
>> > Hmm ...
>> > Do we need this local declaration?
>> > Is there some header we could use?
>>
>> That doesn't appear to be the case, no
>
> Seems like a future opportunity.
>
>> > If not - could we move it out to PrePi.h?
>>
>> Yes, that works for me.
>
> OK, if you fold that in, for the series:
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>

Thanks. Pushed as c6e51751e026..73fcbb48665e

I have taken the liberty of reordering this patch with the one that
clones PrePi for Beagle, so this fix is applied to the clone as well.


>> >> @@ -162,8 +156,7 @@ PrePiMain (
>> >>    PERF_START (NULL, "PEI", NULL, StartTimeStamp);
>> >>
>> >>    // SEC phase needs to run library constructors by hand.
>> >> -  ExtractGuidedSectionLibConstructor ();
>> >> -  LzmaDecompressLibConstructor ();
>> >> +  ProcessLibraryConstructorList ();
>> >>
>> >>    // Assume the FV that contains the SEC (our code) also contains a compressed FV.
>> >>    Status = DecompressFirstFv ();
>> >> --
>> >> 2.11.0
>> >>
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/7] ArmPlatformPkg/MemoryInitPeiLib: don't reserve primary FV in memory
  2017-11-30 15:24 ` [PATCH 5/7] ArmPlatformPkg/MemoryInitPeiLib: don't reserve primary FV in memory Ard Biesheuvel
@ 2017-12-26 21:52   ` Vladimir Olovyannikov
  2017-12-26 23:06     ` Ard Biesheuvel
  2017-12-27  7:37     ` Udit Kumar
  0 siblings, 2 replies; 17+ messages in thread
From: Vladimir Olovyannikov @ 2017-12-26 21:52 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel; +Cc: leif.lindholm

Hi Ard, Meenakshi,

I am having a problem I cannot explain the reason for, with this commit on
an ARM64 platform.

   ArmPlatformPkg/MemoryInitPeiLib: don't reserve primary FV in memory

    Now that PrePi no longer exposes its internal code via special HOBs,
    we can remove the special handling of the primary FV, which needed to
    be reserved so that DXE core could call into the PE/COFF and LZMA
    libraries in the PrePi module.

    Contributed-under: TianoCore Contribution Agreement 1.1
    Signed-off-by: Udit Kumar <udit.kumar@nxp.com>
    Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
    [ardb: updated commit log]
    Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
    Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

If a Shell is built "as is" from the source tree, there are no issues.
However, if I slightly modify Shell.c like in the following patch:

diff --git a/ShellPkg/Application/Shell/Shell.c
b/ShellPkg/Application/Shell/Shell.c
index 577e17311bea..bbbdde8ced96 100644
--- a/ShellPkg/Application/Shell/Shell.c
+++ b/ShellPkg/Application/Shell/Shell.c
@@ -339,6 +339,11 @@ UefiMain (
   EFI_HANDLE                      ConInHandle;
   EFI_SIMPLE_TEXT_INPUT_PROTOCOL  *OldConIn;
   SPLIT_LIST                      *Split;
+  CHAR16                          *DelayStr;
+  CHAR16                          *NoMapStr;
+  UINTN                           DelayVarSize;
+  UINTN                           NoMapVarSize;
+  BOOLEAN                         SilentStart;

   if (PcdGet8(PcdShellSupportLevel) > 3) {
     return (EFI_UNSUPPORTED);
@@ -360,6 +365,7 @@ UefiMain (
   ShellInfoObject.PageBreakEnabled            =
PcdGetBool(PcdShellPageBreakDefault);
   ShellInfoObject.ViewingSettings.InsertMode  =
PcdGetBool(PcdShellInsertModeDefault);
   ShellInfoObject.LogScreenCount              = PcdGet8
(PcdShellScreenLogCount  );
+  SilentStart                                 = FALSE;

   //
   // verify we dont allow for spec violation
@@ -452,6 +458,21 @@ UefiMain (
       goto FreeResources;
     }

+    if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.Delay) {
+      // Command line has priority over the variable
+      Status = ShellFindEnvVarInList(L"startupdelay", &DelayStr,
&DelayVarSize, NULL);
+      if (!EFI_ERROR (Status)) {
+        ShellInfoObject.ShellInitSettings.Delay = ShellStrToUintn
(DelayStr);
+      }
+    }
+
+    if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoMap) {
+      Status = ShellFindEnvVarInList(L"silentstart", &NoMapStr,
&NoMapVarSize, NULL);
+      if (!EFI_ERROR (Status)) {
+        SilentStart = (BOOLEAN)ShellStrToUintn (NoMapStr);
+      }
+    }
+
     //
     // If shell support level is >= 1 create the mappings and paths
     //
@@ -492,7 +513,7 @@ UefiMain (
     //
     // Display the version
     //
-    if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoVersion) {
+    if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoVersion &&
!SilentStart) {
         ShellPrintHiiEx (
           0,
           gST->ConOut->Mode->CursorRow,
@@ -529,7 +550,7 @@ UefiMain (
     //
     // Display the mapping
     //
-    if (PcdGet8(PcdShellSupportLevel) >= 2 &&
!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoMap) {
+    if (PcdGet8(PcdShellSupportLevel) >= 2 &&
!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoMap && !SilentStart) {
       Status = RunCommand(L"map");
       ASSERT_EFI_ERROR(Status);
     }

Shell fails to load.
Here is an excerpt from the debug log:

add-symbol-file
/uefi/Build/StingrayPkg/DEBUG_GCC5/AARCH64/ShellPkg/Application/Shell/Shel
l/DEBUG/Shell.dll 0x88480000
Loading driver at 0x0008847F000 EntryPoint=0x00088480000 Shell.efi
InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 8D095118
ProtectUefiImageCommon - 0x8D08ED40
  - 0x000000008847F000 - 0x0000000000152000
SetUefiImageMemoryAttributes - 0x000000008847F000 - 0x0000000000001000
(0x0000000000004008)
SetUefiImageMemoryAttributes - 0x0000000088480000 - 0x00000000000E6000
(0x0000000000020008)
SetUefiImageMemoryAttributes - 0x0000000088566000 - 0x000000000006B000
(0x0000000000004008)
InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8D088920
InstallProtocolInterface: 752F3136-4E16-4FDC-A22A-E5F46812F4CA 8C71AF98
InstallProtocolInterface: 6302D008-7F9B-4F30-87AC-60C9FEF5DA4E 88566710
--- Blank lines -----
3h
--- Blank lines -----

InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B A3ABE6B398
InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B A3ABE6B398
InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B A3ABE6B398
InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B A3ABE6B398
InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1E18
InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1E18
InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1E18
InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1E18
InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1E18
InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1E18
InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
ASSERT [DxeCore]
/uefi/MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c(300)
: ((BOOLEAN)(0==1))

Here 387477C2-69C7-11D2-8E39-00A0C969723B GUID is
gEfiSimpleTextOutProtocolGuid.

And there is no way to do source-level debug because FV image cannot be
found in memory at the given location.
As soon as I revert this commit
(8ae5fc182941cf9ff7a222eb0a484088a0db8e2e), everything gets back to
normal.
Could you please explain me what I am doing wrong?

Thank you,
Vladimir

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard
Biesheuvel
Sent: Thursday, November 30, 2017 7:25 AM
To: edk2-devel@lists.01.org
Cc: leif.lindholm@linaro.org; Ard Biesheuvel
Subject: [edk2] [PATCH 5/7] ArmPlatformPkg/MemoryInitPeiLib: don't reserve
primary FV in memory

From: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>

Now that PrePi no longer exposes its internal code via special HOBs, we
can remove the special handling of the primary FV, which needed to be
reserved so that DXE core could call into the PE/COFF and LZMA libraries
in the PrePi module.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Udit Kumar <udit.kumar@nxp.com>
Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
[ardb: updated commit log]
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c | 69 --------------------
 1 file changed, 69 deletions(-)

diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
index 2feb11f21d5d..d03214b5df66 100644
--- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
+++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
@@ -70,11 +70,7 @@ MemoryPeim (
 {
   ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable;
   EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;
-  UINT64                       ResourceLength;
   EFI_PEI_HOB_POINTERS         NextHob;
-  EFI_PHYSICAL_ADDRESS         FdTop;
-  EFI_PHYSICAL_ADDRESS         SystemMemoryTop;
-  EFI_PHYSICAL_ADDRESS         ResourceTop;
   BOOLEAN                      Found;

   // Get Virtual Memory Map from the Platform Library @@ -121,71 +117,6
@@ MemoryPeim (
     );
   }

-  //
-  // Reserved the memory space occupied by the firmware volume
-  //
-
-  SystemMemoryTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdSystemMemoryBase)
+ (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdSystemMemorySize);
-  FdTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFdBaseAddress) +
(EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFdSize);
-
-  // EDK2 does not have the concept of boot firmware copied into DRAM. To
avoid the DXE
-  // core to overwrite this area we must mark the region with the
attribute non-present
-  if ((PcdGet64 (PcdFdBaseAddress) >= PcdGet64 (PcdSystemMemoryBase)) &&
(FdTop <= SystemMemoryTop)) {
-    Found = FALSE;
-
-    // Search for System Memory Hob that contains the firmware
-    NextHob.Raw = GetHobList ();
-    while ((NextHob.Raw = GetNextHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR,
NextHob.Raw)) != NULL) {
-      if ((NextHob.ResourceDescriptor->ResourceType ==
EFI_RESOURCE_SYSTEM_MEMORY) &&
-          (PcdGet64 (PcdFdBaseAddress) >=
NextHob.ResourceDescriptor->PhysicalStart) &&
-          (FdTop <= NextHob.ResourceDescriptor->PhysicalStart +
NextHob.ResourceDescriptor->ResourceLength))
-      {
-        ResourceAttributes =
NextHob.ResourceDescriptor->ResourceAttribute;
-        ResourceLength = NextHob.ResourceDescriptor->ResourceLength;
-        ResourceTop = NextHob.ResourceDescriptor->PhysicalStart +
ResourceLength;
-
-        if (PcdGet64 (PcdFdBaseAddress) ==
NextHob.ResourceDescriptor->PhysicalStart) {
-          if (SystemMemoryTop == FdTop) {
-            NextHob.ResourceDescriptor->ResourceAttribute =
ResourceAttributes & ~EFI_RESOURCE_ATTRIBUTE_PRESENT;
-          } else {
-            // Create the System Memory HOB for the firmware with the
non-present attribute
-            BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
-                                        ResourceAttributes &
~EFI_RESOURCE_ATTRIBUTE_PRESENT,
-                                        PcdGet64 (PcdFdBaseAddress),
-                                        PcdGet32 (PcdFdSize));
-
-            // Top of the FD is system memory available for UEFI
-            NextHob.ResourceDescriptor->PhysicalStart +=
PcdGet32(PcdFdSize);
-            NextHob.ResourceDescriptor->ResourceLength -=
PcdGet32(PcdFdSize);
-          }
-        } else {
-          // Create the System Memory HOB for the firmware with the
non-present attribute
-          BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
-                                      ResourceAttributes &
~EFI_RESOURCE_ATTRIBUTE_PRESENT,
-                                      PcdGet64 (PcdFdBaseAddress),
-                                      PcdGet32 (PcdFdSize));
-
-          // Update the HOB
-          NextHob.ResourceDescriptor->ResourceLength = PcdGet64
(PcdFdBaseAddress) - NextHob.ResourceDescriptor->PhysicalStart;
-
-          // If there is some memory available on the top of the FD then
create a HOB
-          if (FdTop < NextHob.ResourceDescriptor->PhysicalStart +
ResourceLength) {
-            // Create the System Memory HOB for the remaining region (top
of the FD)
-            BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
-                                        ResourceAttributes,
-                                        FdTop,
-                                        ResourceTop - FdTop);
-          }
-        }
-        Found = TRUE;
-        break;
-      }
-      NextHob.Raw = GET_NEXT_HOB (NextHob);
-    }
-
-    ASSERT(Found);
-  }
-
   // Build Memory Allocation Hob
   InitMmu (MemoryTable);

--
2.11.0

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/7] ArmPlatformPkg/MemoryInitPeiLib: don't reserve primary FV in memory
  2017-12-26 21:52   ` Vladimir Olovyannikov
@ 2017-12-26 23:06     ` Ard Biesheuvel
  2017-12-27  1:58       ` Vladimir Olovyannikov
  2017-12-27  2:01       ` Vladimir Olovyannikov
  2017-12-27  7:37     ` Udit Kumar
  1 sibling, 2 replies; 17+ messages in thread
From: Ard Biesheuvel @ 2017-12-26 23:06 UTC (permalink / raw)
  To: Vladimir Olovyannikov; +Cc: edk2-devel@lists.01.org, Leif Lindholm

On 26 December 2017 at 21:52, Vladimir Olovyannikov
<vladimir.olovyannikov@broadcom.com> wrote:
> Hi Ard, Meenakshi,
>
> I am having a problem I cannot explain the reason for, with this commit on
> an ARM64 platform.
>
>    ArmPlatformPkg/MemoryInitPeiLib: don't reserve primary FV in memory
>
>     Now that PrePi no longer exposes its internal code via special HOBs,
>     we can remove the special handling of the primary FV, which needed to
>     be reserved so that DXE core could call into the PE/COFF and LZMA
>     libraries in the PrePi module.
>
>     Contributed-under: TianoCore Contribution Agreement 1.1
>     Signed-off-by: Udit Kumar <udit.kumar@nxp.com>
>     Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
>     [ardb: updated commit log]
>     Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>     Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>
> If a Shell is built "as is" from the source tree, there are no issues.
> However, if I slightly modify Shell.c like in the following patch:
>
> diff --git a/ShellPkg/Application/Shell/Shell.c
> b/ShellPkg/Application/Shell/Shell.c
> index 577e17311bea..bbbdde8ced96 100644
> --- a/ShellPkg/Application/Shell/Shell.c
> +++ b/ShellPkg/Application/Shell/Shell.c
> @@ -339,6 +339,11 @@ UefiMain (
>    EFI_HANDLE                      ConInHandle;
>    EFI_SIMPLE_TEXT_INPUT_PROTOCOL  *OldConIn;
>    SPLIT_LIST                      *Split;
> +  CHAR16                          *DelayStr;
> +  CHAR16                          *NoMapStr;
> +  UINTN                           DelayVarSize;
> +  UINTN                           NoMapVarSize;
> +  BOOLEAN                         SilentStart;
>
>    if (PcdGet8(PcdShellSupportLevel) > 3) {
>      return (EFI_UNSUPPORTED);
> @@ -360,6 +365,7 @@ UefiMain (
>    ShellInfoObject.PageBreakEnabled            =
> PcdGetBool(PcdShellPageBreakDefault);
>    ShellInfoObject.ViewingSettings.InsertMode  =
> PcdGetBool(PcdShellInsertModeDefault);
>    ShellInfoObject.LogScreenCount              = PcdGet8
> (PcdShellScreenLogCount  );
> +  SilentStart                                 = FALSE;
>
>    //
>    // verify we dont allow for spec violation
> @@ -452,6 +458,21 @@ UefiMain (
>        goto FreeResources;
>      }
>
> +    if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.Delay) {
> +      // Command line has priority over the variable
> +      Status = ShellFindEnvVarInList(L"startupdelay", &DelayStr,
> &DelayVarSize, NULL);
> +      if (!EFI_ERROR (Status)) {
> +        ShellInfoObject.ShellInitSettings.Delay = ShellStrToUintn
> (DelayStr);
> +      }
> +    }
> +
> +    if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoMap) {
> +      Status = ShellFindEnvVarInList(L"silentstart", &NoMapStr,
> &NoMapVarSize, NULL);
> +      if (!EFI_ERROR (Status)) {
> +        SilentStart = (BOOLEAN)ShellStrToUintn (NoMapStr);
> +      }
> +    }
> +
>      //
>      // If shell support level is >= 1 create the mappings and paths
>      //
> @@ -492,7 +513,7 @@ UefiMain (
>      //
>      // Display the version
>      //
> -    if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoVersion) {
> +    if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoVersion &&
> !SilentStart) {
>          ShellPrintHiiEx (
>            0,
>            gST->ConOut->Mode->CursorRow,
> @@ -529,7 +550,7 @@ UefiMain (
>      //
>      // Display the mapping
>      //
> -    if (PcdGet8(PcdShellSupportLevel) >= 2 &&
> !ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoMap) {
> +    if (PcdGet8(PcdShellSupportLevel) >= 2 &&
> !ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoMap && !SilentStart) {
>        Status = RunCommand(L"map");
>        ASSERT_EFI_ERROR(Status);
>      }
>
> Shell fails to load.
> Here is an excerpt from the debug log:
>
> add-symbol-file
> /uefi/Build/StingrayPkg/DEBUG_GCC5/AARCH64/ShellPkg/Application/Shell/Shel
> l/DEBUG/Shell.dll 0x88480000
> Loading driver at 0x0008847F000 EntryPoint=0x00088480000 Shell.efi
> InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 8D095118
> ProtectUefiImageCommon - 0x8D08ED40
>   - 0x000000008847F000 - 0x0000000000152000
> SetUefiImageMemoryAttributes - 0x000000008847F000 - 0x0000000000001000
> (0x0000000000004008)
> SetUefiImageMemoryAttributes - 0x0000000088480000 - 0x00000000000E6000
> (0x0000000000020008)
> SetUefiImageMemoryAttributes - 0x0000000088566000 - 0x000000000006B000
> (0x0000000000004008)
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8D088920
> InstallProtocolInterface: 752F3136-4E16-4FDC-A22A-E5F46812F4CA 8C71AF98
> InstallProtocolInterface: 6302D008-7F9B-4F30-87AC-60C9FEF5DA4E 88566710
> --- Blank lines -----
> 3h
> --- Blank lines -----
>
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B A3ABE6B398
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B A3ABE6B398
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B A3ABE6B398
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B A3ABE6B398
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1E18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1E18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1E18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1E18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1E18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1E18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
> ASSERT [DxeCore]
> /uefi/MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c(300)
> : ((BOOLEAN)(0==1))
>
> Here 387477C2-69C7-11D2-8E39-00A0C969723B GUID is
> gEfiSimpleTextOutProtocolGuid.
>
> And there is no way to do source-level debug because FV image cannot be
> found in memory at the given location.
> As soon as I revert this commit
> (8ae5fc182941cf9ff7a222eb0a484088a0db8e2e), everything gets back to
> normal.
> Could you please explain me what I am doing wrong?
>

Does this help?

--- a/ArmPlatformPkg/PlatformPei/PlatformPeiLib.c
+++ b/ArmPlatformPkg/PlatformPei/PlatformPeiLib.c
@@ -24,7 +24,7 @@ PlatformPeim (
   VOID
   )
 {
-  BuildFvHob (PcdGet64 (PcdFvBaseAddress), PcdGet32 (PcdFvSize));
+  //BuildFvHob (PcdGet64 (PcdFvBaseAddress), PcdGet32 (PcdFvSize));

   return EFI_SUCCESS;
 }

(I assume you are using PrePi, and have nothing except the PrePi
module in the primary FV)


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/7] ArmPlatformPkg/MemoryInitPeiLib: don't reserve primary FV in memory
  2017-12-26 23:06     ` Ard Biesheuvel
@ 2017-12-27  1:58       ` Vladimir Olovyannikov
  2017-12-27  2:01       ` Vladimir Olovyannikov
  1 sibling, 0 replies; 17+ messages in thread
From: Vladimir Olovyannikov @ 2017-12-27  1:58 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel, Leif Lindholm

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Tuesday, December 26, 2017 3:07 PM
> To: Vladimir Olovyannikov
> Cc: edk2-devel@lists.01.org; Leif Lindholm
> Subject: Re: [edk2] [PATCH 5/7] ArmPlatformPkg/MemoryInitPeiLib: don't
> reserve primary FV in memory
>
> On 26 December 2017 at 21:52, Vladimir Olovyannikov
> <vladimir.olovyannikov@broadcom.com> wrote:
> > Hi Ard, Meenakshi,
> >
> > I am having a problem I cannot explain the reason for, with this
> > commit on an ARM64 platform.
> >
> >    ArmPlatformPkg/MemoryInitPeiLib: don't reserve primary FV in memory
> >
> >     Now that PrePi no longer exposes its internal code via special HOBs,
> >     we can remove the special handling of the primary FV, which needed
> > to
> >     be reserved so that DXE core could call into the PE/COFF and LZMA
> >     libraries in the PrePi module.
> >
> >     Contributed-under: TianoCore Contribution Agreement 1.1
> >     Signed-off-by: Udit Kumar <udit.kumar@nxp.com>
> >     Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> >     [ardb: updated commit log]
> >     Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >     Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> >
> > If a Shell is built "as is" from the source tree, there are no issues.
> > However, if I slightly modify Shell.c like in the following patch:
> >
> > diff --git a/ShellPkg/Application/Shell/Shell.c
> > b/ShellPkg/Application/Shell/Shell.c
> > index 577e17311bea..bbbdde8ced96 100644
> > --- a/ShellPkg/Application/Shell/Shell.c
> > +++ b/ShellPkg/Application/Shell/Shell.c
> > @@ -339,6 +339,11 @@ UefiMain (
> >    EFI_HANDLE                      ConInHandle;
> >    EFI_SIMPLE_TEXT_INPUT_PROTOCOL  *OldConIn;
> >    SPLIT_LIST                      *Split;
> > +  CHAR16                          *DelayStr;
> > +  CHAR16                          *NoMapStr;
> > +  UINTN                           DelayVarSize;
> > +  UINTN                           NoMapVarSize;
> > +  BOOLEAN                         SilentStart;
> >
> >    if (PcdGet8(PcdShellSupportLevel) > 3) {
> >      return (EFI_UNSUPPORTED);
> > @@ -360,6 +365,7 @@ UefiMain (
> >    ShellInfoObject.PageBreakEnabled            =
> > PcdGetBool(PcdShellPageBreakDefault);
> >    ShellInfoObject.ViewingSettings.InsertMode  =
> > PcdGetBool(PcdShellInsertModeDefault);
> >    ShellInfoObject.LogScreenCount              = PcdGet8
> > (PcdShellScreenLogCount  );
> > +  SilentStart                                 = FALSE;
> >
> >    //
> >    // verify we dont allow for spec violation @@ -452,6 +458,21 @@
> > UefiMain (
> >        goto FreeResources;
> >      }
> >
> > +    if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.Delay) {
> > +      // Command line has priority over the variable
> > +      Status = ShellFindEnvVarInList(L"startupdelay", &DelayStr,
> > &DelayVarSize, NULL);
> > +      if (!EFI_ERROR (Status)) {
> > +        ShellInfoObject.ShellInitSettings.Delay = ShellStrToUintn
> > (DelayStr);
> > +      }
> > +    }
> > +
> > +    if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoMap) {
> > +      Status = ShellFindEnvVarInList(L"silentstart", &NoMapStr,
> > &NoMapVarSize, NULL);
> > +      if (!EFI_ERROR (Status)) {
> > +        SilentStart = (BOOLEAN)ShellStrToUintn (NoMapStr);
> > +      }
> > +    }
> > +
> >      //
> >      // If shell support level is >= 1 create the mappings and paths
> >      //
> > @@ -492,7 +513,7 @@ UefiMain (
> >      //
> >      // Display the version
> >      //
> > -    if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoVersion) {
> > +    if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoVersion &&
> > !SilentStart) {
> >          ShellPrintHiiEx (
> >            0,
> >            gST->ConOut->Mode->CursorRow, @@ -529,7 +550,7 @@ UefiMain
> > (
> >      //
> >      // Display the mapping
> >      //
> > -    if (PcdGet8(PcdShellSupportLevel) >= 2 &&
> > !ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoMap) {
> > +    if (PcdGet8(PcdShellSupportLevel) >= 2 &&
> > !ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoMap && !SilentStart)
> > {
> >        Status = RunCommand(L"map");
> >        ASSERT_EFI_ERROR(Status);
> >      }
> >
> > Shell fails to load.
> > Here is an excerpt from the debug log:
> >
> > add-symbol-file
> >
> /uefi/Build/StingrayPkg/DEBUG_GCC5/AARCH64/ShellPkg/Application/Shell/
> > Shel
> > l/DEBUG/Shell.dll 0x88480000
> > Loading driver at 0x0008847F000 EntryPoint=0x00088480000 Shell.efi
> > InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF
> > 8D095118 ProtectUefiImageCommon - 0x8D08ED40
> >   - 0x000000008847F000 - 0x0000000000152000
> > SetUefiImageMemoryAttributes - 0x000000008847F000 -
> 0x0000000000001000
> > (0x0000000000004008)
> > SetUefiImageMemoryAttributes - 0x0000000088480000 -
> 0x00000000000E6000
> > (0x0000000000020008)
> > SetUefiImageMemoryAttributes - 0x0000000088566000 -
> 0x000000000006B000
> > (0x0000000000004008)
> > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > 8D088920
> > InstallProtocolInterface: 752F3136-4E16-4FDC-A22A-E5F46812F4CA
> > 8C71AF98
> > InstallProtocolInterface: 6302D008-7F9B-4F30-87AC-60C9FEF5DA4E
> > 88566710
> > --- Blank lines -----
> > 3h
> > --- Blank lines -----
> >
> > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > 8C0A1B18
> > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > A3ABE6B398
> > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > 8C0A1B18
> > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > A3ABE6B398
> > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > 8C0A1B18
> > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > A3ABE6B398
> > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > 8C0A1B18
> > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > A3ABE6B398
> > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > 8C0A1B18
> > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > 8C0A1B18
> > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > 8C0A1E18
> > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > 8C0A1B18
> > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > 8C0A1E18
> > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > 8C0A1B18
> > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > 8C0A1E18
> > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > 8C0A1B18
> > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > 8C0A1E18
> > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > 8C0A1B18
> > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > 8C0A1E18
> > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > 8C0A1B18
> > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > 8C0A1E18
> > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > 8C0A1B18 ASSERT [DxeCore]
> >
> /uefi/MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c(
> > 300)
> > : ((BOOLEAN)(0==1))
> >
> > Here 387477C2-69C7-11D2-8E39-00A0C969723B GUID is
> > gEfiSimpleTextOutProtocolGuid.
> >
> > And there is no way to do source-level debug because FV image cannot
> > be found in memory at the given location.
> > As soon as I revert this commit
> > (8ae5fc182941cf9ff7a222eb0a484088a0db8e2e), everything gets back to
> > normal.
> > Could you please explain me what I am doing wrong?
> >
>
> Does this help?
>
> --- a/ArmPlatformPkg/PlatformPei/PlatformPeiLib.c
> +++ b/ArmPlatformPkg/PlatformPei/PlatformPeiLib.c
> @@ -24,7 +24,7 @@ PlatformPeim (
>    VOID
>    )
>  {
> -  BuildFvHob (PcdGet64 (PcdFvBaseAddress), PcdGet32 (PcdFvSize));
> +  //BuildFvHob (PcdGet64 (PcdFvBaseAddress), PcdGet32 (PcdFvSize));
>
>    return EFI_SUCCESS;
>  }
>
> (I assume you are using PrePi, and have nothing except the PrePi module in
> the primary FV)
Thanks for response Ard,
No, this does not help.
In ArmPlatformGetVirtualMemoryMap () the platform reads a Pcd describing
SDRAM memory configuration (regions),
For each described region it creates a memory resource HOB like this:

if (PrepareMemoryResourceHob (
        MyDDRInfo[Index].Address,
        MyDDRInfo[Index].Size,
        MyDDRInfo[Index].Size + 1,
        Reserve ? EFI_RESOURCE_MEMORY_RESERVED : EFI_RESOURCE_SYSTEM_MEMORY
      )) {
        UINTN SizeGB;

        TotalHighMemAdded += MyDDRInfo[Index].Size;
        SizeGB = MyDDRInfo[Index].Size >> 30;
        DEBUG((
          EFI_D_INFO,
          "HighMemMap: Added DDR %a area (%d). Start address: 0x%llx, size
%llu %a (0x%llx)\n",
          Reserve ? "reserved" : "highmem",
          Index + 1,
          MyDDRInfo[Index].Address,
          SizeGB ? SizeGB : MyDDRInfo[Index].Size >> 20,
          SizeGB ? "GiB" : "MiB",
          MyDDRInfo[Index].Size
        ));
        Index++;
      }
     } else {
         MyDDRInfo[Index].Address = 0;
         MyDDRInfo[Index].Size = 0;
     }

Thus it reports described and filled in areas of memory like this:
HighMemMap: Added DDR highmem area (1). Start address: 0x8F101000, size 1
GiB (0x70EFF000)
HighMemMap: Added DDR highmem area (2). Start address: 0x880000000, size 14
GiB (0x380000000)
HighMemMap: Added DDR highmem area (3). Start address: 0x9000000000, size 16
GiB (0x400000000)
HighMemMap: Added DDR highmem area (4). Start address: 0xA000000000, size 16
GiB (0x400000000)
HighMemMap: Added DDR reserved area (1). Start address: 0x8F000000, size 1
MiB (0x101000)

IS this not the right way to describe memory HOBs?

With your proposed change assertion happens very early, here:

ASSERT_EFI_ERROR (Status = Not Found)
ASSERT [ArmPlatformPrePiMPCore] /uefi/ArmPlatformPkg/PrePi/PrePi.c(157):
!EFI_ERROR (Status)

So it cannot DecompressFirstFv().
If I don't apply your suggested change and revert the commit I mentioned
earlier, it works fine...

Thank you,
Vladimir


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/7] ArmPlatformPkg/MemoryInitPeiLib: don't reserve primary FV in memory
  2017-12-26 23:06     ` Ard Biesheuvel
  2017-12-27  1:58       ` Vladimir Olovyannikov
@ 2017-12-27  2:01       ` Vladimir Olovyannikov
  1 sibling, 0 replies; 17+ messages in thread
From: Vladimir Olovyannikov @ 2017-12-27  2:01 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel, Leif Lindholm

> -----Original Message-----
> From: Vladimir Olovyannikov [mailto:vladimir.olovyannikov@broadcom.com]
> Sent: Tuesday, December 26, 2017 5:59 PM
> To: 'Ard Biesheuvel'
> Cc: 'edk2-devel@lists.01.org'; 'Leif Lindholm'
> Subject: RE: [edk2] [PATCH 5/7] ArmPlatformPkg/MemoryInitPeiLib: don't
> reserve primary FV in memory
>
> > -----Original Message-----
> > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > Sent: Tuesday, December 26, 2017 3:07 PM
> > To: Vladimir Olovyannikov
> > Cc: edk2-devel@lists.01.org; Leif Lindholm
> > Subject: Re: [edk2] [PATCH 5/7] ArmPlatformPkg/MemoryInitPeiLib: don't
> > reserve primary FV in memory
> >
> > On 26 December 2017 at 21:52, Vladimir Olovyannikov
> > <vladimir.olovyannikov@broadcom.com> wrote:
> > > Hi Ard, Meenakshi,
> > >
> > > I am having a problem I cannot explain the reason for, with this
> > > commit on an ARM64 platform.
> > >
> > >    ArmPlatformPkg/MemoryInitPeiLib: don't reserve primary FV in
> > > memory
> > >
> > >     Now that PrePi no longer exposes its internal code via special
> > > HOBs,
> > >     we can remove the special handling of the primary FV, which needed
> > > to
> > >     be reserved so that DXE core could call into the PE/COFF and LZMA
> > >     libraries in the PrePi module.
> > >
> > >     Contributed-under: TianoCore Contribution Agreement 1.1
> > >     Signed-off-by: Udit Kumar <udit.kumar@nxp.com>
> > >     Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> > >     [ardb: updated commit log]
> > >     Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > >     Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> > >
> > > If a Shell is built "as is" from the source tree, there are no issues.
> > > However, if I slightly modify Shell.c like in the following patch:
> > >
> > > diff --git a/ShellPkg/Application/Shell/Shell.c
> > > b/ShellPkg/Application/Shell/Shell.c
> > > index 577e17311bea..bbbdde8ced96 100644
> > > --- a/ShellPkg/Application/Shell/Shell.c
> > > +++ b/ShellPkg/Application/Shell/Shell.c
> > > @@ -339,6 +339,11 @@ UefiMain (
> > >    EFI_HANDLE                      ConInHandle;
> > >    EFI_SIMPLE_TEXT_INPUT_PROTOCOL  *OldConIn;
> > >    SPLIT_LIST                      *Split;
> > > +  CHAR16                          *DelayStr;
> > > +  CHAR16                          *NoMapStr;
> > > +  UINTN                           DelayVarSize;
> > > +  UINTN                           NoMapVarSize;
> > > +  BOOLEAN                         SilentStart;
> > >
> > >    if (PcdGet8(PcdShellSupportLevel) > 3) {
> > >      return (EFI_UNSUPPORTED);
> > > @@ -360,6 +365,7 @@ UefiMain (
> > >    ShellInfoObject.PageBreakEnabled            =
> > > PcdGetBool(PcdShellPageBreakDefault);
> > >    ShellInfoObject.ViewingSettings.InsertMode  =
> > > PcdGetBool(PcdShellInsertModeDefault);
> > >    ShellInfoObject.LogScreenCount              = PcdGet8
> > > (PcdShellScreenLogCount  );
> > > +  SilentStart                                 = FALSE;
> > >
> > >    //
> > >    // verify we dont allow for spec violation @@ -452,6 +458,21 @@
> > > UefiMain (
> > >        goto FreeResources;
> > >      }
> > >
> > > +    if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.Delay) {
> > > +      // Command line has priority over the variable
> > > +      Status = ShellFindEnvVarInList(L"startupdelay", &DelayStr,
> > > &DelayVarSize, NULL);
> > > +      if (!EFI_ERROR (Status)) {
> > > +        ShellInfoObject.ShellInitSettings.Delay = ShellStrToUintn
> > > (DelayStr);
> > > +      }
> > > +    }
> > > +
> > > +    if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoMap) {
> > > +      Status = ShellFindEnvVarInList(L"silentstart", &NoMapStr,
> > > &NoMapVarSize, NULL);
> > > +      if (!EFI_ERROR (Status)) {
> > > +        SilentStart = (BOOLEAN)ShellStrToUintn (NoMapStr);
> > > +      }
> > > +    }
> > > +
> > >      //
> > >      // If shell support level is >= 1 create the mappings and paths
> > >      //
> > > @@ -492,7 +513,7 @@ UefiMain (
> > >      //
> > >      // Display the version
> > >      //
> > > -    if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoVersion) {
> > > +    if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoVersion
> > > + &&
> > > !SilentStart) {
> > >          ShellPrintHiiEx (
> > >            0,
> > >            gST->ConOut->Mode->CursorRow, @@ -529,7 +550,7 @@
> > > UefiMain (
> > >      //
> > >      // Display the mapping
> > >      //
> > > -    if (PcdGet8(PcdShellSupportLevel) >= 2 &&
> > > !ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoMap) {
> > > +    if (PcdGet8(PcdShellSupportLevel) >= 2 &&
> > > !ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoMap &&
> > > !SilentStart) {
> > >        Status = RunCommand(L"map");
> > >        ASSERT_EFI_ERROR(Status);
> > >      }
> > >
> > > Shell fails to load.
> > > Here is an excerpt from the debug log:
> > >
> > > add-symbol-file
> > >
> >
> /uefi/Build/StingrayPkg/DEBUG_GCC5/AARCH64/ShellPkg/Application/Shell/
> > > Shel
> > > l/DEBUG/Shell.dll 0x88480000
> > > Loading driver at 0x0008847F000 EntryPoint=0x00088480000 Shell.efi
> > > InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF
> > > 8D095118 ProtectUefiImageCommon - 0x8D08ED40
> > >   - 0x000000008847F000 - 0x0000000000152000
> > > SetUefiImageMemoryAttributes - 0x000000008847F000 -
> > 0x0000000000001000
> > > (0x0000000000004008)
> > > SetUefiImageMemoryAttributes - 0x0000000088480000 -
> > 0x00000000000E6000
> > > (0x0000000000020008)
> > > SetUefiImageMemoryAttributes - 0x0000000088566000 -
> > 0x000000000006B000
> > > (0x0000000000004008)
> > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > > 8D088920
> > > InstallProtocolInterface: 752F3136-4E16-4FDC-A22A-E5F46812F4CA
> > > 8C71AF98
> > > InstallProtocolInterface: 6302D008-7F9B-4F30-87AC-60C9FEF5DA4E
> > > 88566710
> > > --- Blank lines -----
> > > 3h
> > > --- Blank lines -----
> > >
> > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > > 8C0A1B18
> > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > > A3ABE6B398
> > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > > 8C0A1B18
> > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > > A3ABE6B398
> > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > > 8C0A1B18
> > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > > A3ABE6B398
> > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > > 8C0A1B18
> > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > > A3ABE6B398
> > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > > 8C0A1B18
> > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > > 8C0A1B18
> > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > > 8C0A1E18
> > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > > 8C0A1B18
> > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > > 8C0A1E18
> > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > > 8C0A1B18
> > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > > 8C0A1E18
> > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > > 8C0A1B18
> > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > > 8C0A1E18
> > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > > 8C0A1B18
> > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > > 8C0A1E18
> > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > > 8C0A1B18
> > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > > 8C0A1E18
> > > InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> > > 8C0A1B18 ASSERT [DxeCore]
> > >
> >
> /uefi/MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c(
> > > 300)
> > > : ((BOOLEAN)(0==1))
> > >
> > > Here 387477C2-69C7-11D2-8E39-00A0C969723B GUID is
> > > gEfiSimpleTextOutProtocolGuid.
> > >
> > > And there is no way to do source-level debug because FV image cannot
> > > be found in memory at the given location.
> > > As soon as I revert this commit
> > > (8ae5fc182941cf9ff7a222eb0a484088a0db8e2e), everything gets back to
> > > normal.
> > > Could you please explain me what I am doing wrong?
> > >
> >
> > Does this help?
> >
> > --- a/ArmPlatformPkg/PlatformPei/PlatformPeiLib.c
> > +++ b/ArmPlatformPkg/PlatformPei/PlatformPeiLib.c
> > @@ -24,7 +24,7 @@ PlatformPeim (
> >    VOID
> >    )
> >  {
> > -  BuildFvHob (PcdGet64 (PcdFvBaseAddress), PcdGet32 (PcdFvSize));
> > +  //BuildFvHob (PcdGet64 (PcdFvBaseAddress), PcdGet32 (PcdFvSize));
> >
> >    return EFI_SUCCESS;
> >  }
> >
> > (I assume you are using PrePi, and have nothing except the PrePi
> > module in the primary FV)
> Thanks for response Ard,
> No, this does not help.
> In ArmPlatformGetVirtualMemoryMap () the platform reads a Pcd describing
> SDRAM memory configuration (regions), For each described region it creates
> a memory resource HOB like this:
>
> if (PrepareMemoryResourceHob (
>         MyDDRInfo[Index].Address,
>         MyDDRInfo[Index].Size,
>         MyDDRInfo[Index].Size + 1,
>         Reserve ? EFI_RESOURCE_MEMORY_RESERVED :
> EFI_RESOURCE_SYSTEM_MEMORY
>       )) {
>         UINTN SizeGB;
>
>         TotalHighMemAdded += MyDDRInfo[Index].Size;
>         SizeGB = MyDDRInfo[Index].Size >> 30;
>         DEBUG((
>           EFI_D_INFO,
>           "HighMemMap: Added DDR %a area (%d). Start address: 0x%llx, size
> %llu %a (0x%llx)\n",
>           Reserve ? "reserved" : "highmem",
>           Index + 1,
>           MyDDRInfo[Index].Address,
>           SizeGB ? SizeGB : MyDDRInfo[Index].Size >> 20,
>           SizeGB ? "GiB" : "MiB",
>           MyDDRInfo[Index].Size
>         ));
>         Index++;
>       }
>      } else {
>          MyDDRInfo[Index].Address = 0;
>          MyDDRInfo[Index].Size = 0;
>      }
>
> Thus it reports described and filled in areas of memory like this:
> HighMemMap: Added DDR highmem area (1). Start address: 0x8F101000, size
> 1 GiB (0x70EFF000)
> HighMemMap: Added DDR highmem area (2). Start address: 0x880000000,
> size 14 GiB (0x380000000)
> HighMemMap: Added DDR highmem area (3). Start address: 0x9000000000,
> size 16 GiB (0x400000000)
> HighMemMap: Added DDR highmem area (4). Start address: 0xA000000000,
> size 16 GiB (0x400000000)
> HighMemMap: Added DDR reserved area (1). Start address: 0x8F000000, size
> 1 MiB (0x101000)
>
> IS this not the right way to describe memory HOBs?
>
> With your proposed change assertion happens very early, here:
>
> ASSERT_EFI_ERROR (Status = Not Found)
> ASSERT [ArmPlatformPrePiMPCore]
> /uefi/ArmPlatformPkg/PrePi/PrePi.c(157): !EFI_ERROR (Status)
>
> So it cannot DecompressFirstFv().
> If I don't apply your suggested change and revert the commit I mentioned
> earlier, it works fine...
>
To clarify, PreparememoryResourceHob looks like this:
BOOLEAN
PrepareMemoryResourceHob (
  IN EFI_PHYSICAL_ADDRESS       Address,
  IN UINT64                     MemSize,
  IN UINT64                     MaxAllowedSize,
  UINTN                         MemType
  )
{
  BOOLEAN HobCreated;

  HobCreated = FALSE;

  if ((MemSize > 0) && (MemSize <= MaxAllowedSize)) {
    // Additional memory is available. Declare it.
    EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;

    ResourceAttributes = SYSTEM_MEMORY_ATTRS;
    if (MemType != EFI_RESOURCE_SYSTEM_MEMORY) {
      ResourceAttributes = RESERVED_MEMORY_ATTRS;
    }

    BuildResourceDescriptorHob (
      MemType,
      ResourceAttributes,
      Address,
      MemSize);

    HobCreated = TRUE;
  }

  return HobCreated;
}

And SYSTEM_MEMORY_ATTRS:

#define RESERVED_MEMORY_ATTRS \
    EFI_RESOURCE_ATTRIBUTE_PRESENT                  | \
    EFI_RESOURCE_ATTRIBUTE_WRITE_PROTECTED          | \
    EFI_RESOURCE_ATTRIBUTE_WRITE_PROTECTABLE        | \
    EFI_RESOURCE_ATTRIBUTE_INITIALIZED              | \
    EFI_RESOURCE_ATTRIBUTE_READ_PROTECTED           | \
    EFI_RESOURCE_ATTRIBUTE_READ_PROTECTABLE         | \
    EFI_RESOURCE_ATTRIBUTE_UNCACHEABLE              | \
    EFI_RESOURCE_ATTRIBUTE_TESTED

> Thank you,
> Vladimir


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/7] ArmPlatformPkg/MemoryInitPeiLib: don't reserve primary FV in memory
  2017-12-26 21:52   ` Vladimir Olovyannikov
  2017-12-26 23:06     ` Ard Biesheuvel
@ 2017-12-27  7:37     ` Udit Kumar
  1 sibling, 0 replies; 17+ messages in thread
From: Udit Kumar @ 2017-12-27  7:37 UTC (permalink / raw)
  To: Vladimir Olovyannikov, Ard Biesheuvel, edk2-devel@lists.01.org
  Cc: leif.lindholm@linaro.org

Hi Vladimir
How re-allocation or say drivers are dispatched on your system. 
Could you check addresses, where FV is kept and where this is getting dispatched 

Thx 
Udit

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Vladimir Olovyannikov
> Sent: Wednesday, December 27, 2017 3:22 AM
> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org
> Cc: leif.lindholm@linaro.org
> Subject: Re: [edk2] [PATCH 5/7] ArmPlatformPkg/MemoryInitPeiLib: don't
> reserve primary FV in memory
> 
> Hi Ard, Meenakshi,
> 
> I am having a problem I cannot explain the reason for, with this commit on
> an ARM64 platform.
> 
>    ArmPlatformPkg/MemoryInitPeiLib: don't reserve primary FV in memory
> 
>     Now that PrePi no longer exposes its internal code via special HOBs,
>     we can remove the special handling of the primary FV, which needed to
>     be reserved so that DXE core could call into the PE/COFF and LZMA
>     libraries in the PrePi module.
> 
>     Contributed-under: TianoCore Contribution Agreement 1.1
>     Signed-off-by: Udit Kumar <udit.kumar@nxp.com>
>     Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
>     [ardb: updated commit log]
>     Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>     Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> 
> If a Shell is built "as is" from the source tree, there are no issues.
> However, if I slightly modify Shell.c like in the following patch:
> 
> diff --git a/ShellPkg/Application/Shell/Shell.c
> b/ShellPkg/Application/Shell/Shell.c
> index 577e17311bea..bbbdde8ced96 100644
> --- a/ShellPkg/Application/Shell/Shell.c
> +++ b/ShellPkg/Application/Shell/Shell.c
> @@ -339,6 +339,11 @@ UefiMain (
>    EFI_HANDLE                      ConInHandle;
>    EFI_SIMPLE_TEXT_INPUT_PROTOCOL  *OldConIn;
>    SPLIT_LIST                      *Split;
> +  CHAR16                          *DelayStr;
> +  CHAR16                          *NoMapStr;
> +  UINTN                           DelayVarSize;
> +  UINTN                           NoMapVarSize;
> +  BOOLEAN                         SilentStart;
> 
>    if (PcdGet8(PcdShellSupportLevel) > 3) {
>      return (EFI_UNSUPPORTED);
> @@ -360,6 +365,7 @@ UefiMain (
>    ShellInfoObject.PageBreakEnabled            =
> PcdGetBool(PcdShellPageBreakDefault);
>    ShellInfoObject.ViewingSettings.InsertMode  =
> PcdGetBool(PcdShellInsertModeDefault);
>    ShellInfoObject.LogScreenCount              = PcdGet8
> (PcdShellScreenLogCount  );
> +  SilentStart                                 = FALSE;
> 
>    //
>    // verify we dont allow for spec violation @@ -452,6 +458,21 @@ UefiMain
> (
>        goto FreeResources;
>      }
> 
> +    if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.Delay) {
> +      // Command line has priority over the variable
> +      Status = ShellFindEnvVarInList(L"startupdelay", &DelayStr,
> &DelayVarSize, NULL);
> +      if (!EFI_ERROR (Status)) {
> +        ShellInfoObject.ShellInitSettings.Delay = ShellStrToUintn
> (DelayStr);
> +      }
> +    }
> +
> +    if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoMap) {
> +      Status = ShellFindEnvVarInList(L"silentstart", &NoMapStr,
> &NoMapVarSize, NULL);
> +      if (!EFI_ERROR (Status)) {
> +        SilentStart = (BOOLEAN)ShellStrToUintn (NoMapStr);
> +      }
> +    }
> +
>      //
>      // If shell support level is >= 1 create the mappings and paths
>      //
> @@ -492,7 +513,7 @@ UefiMain (
>      //
>      // Display the version
>      //
> -    if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoVersion) {
> +    if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoVersion &&
> !SilentStart) {
>          ShellPrintHiiEx (
>            0,
>            gST->ConOut->Mode->CursorRow, @@ -529,7 +550,7 @@ UefiMain (
>      //
>      // Display the mapping
>      //
> -    if (PcdGet8(PcdShellSupportLevel) >= 2 &&
> !ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoMap) {
> +    if (PcdGet8(PcdShellSupportLevel) >= 2 &&
> !ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoMap && !SilentStart) {
>        Status = RunCommand(L"map");
>        ASSERT_EFI_ERROR(Status);
>      }
> 
> Shell fails to load.
> Here is an excerpt from the debug log:
> 
> add-symbol-file
> /uefi/Build/StingrayPkg/DEBUG_GCC5/AARCH64/ShellPkg/Application/Shell/
> Shel
> l/DEBUG/Shell.dll 0x88480000
> Loading driver at 0x0008847F000 EntryPoint=0x00088480000 Shell.efi
> InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 8D095118
> ProtectUefiImageCommon - 0x8D08ED40
>   - 0x000000008847F000 - 0x0000000000152000
> SetUefiImageMemoryAttributes - 0x000000008847F000 -
> 0x0000000000001000
> (0x0000000000004008)
> SetUefiImageMemoryAttributes - 0x0000000088480000 -
> 0x00000000000E6000
> (0x0000000000020008)
> SetUefiImageMemoryAttributes - 0x0000000088566000 -
> 0x000000000006B000
> (0x0000000000004008)
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8D088920
> InstallProtocolInterface: 752F3136-4E16-4FDC-A22A-E5F46812F4CA 8C71AF98
> InstallProtocolInterface: 6302D008-7F9B-4F30-87AC-60C9FEF5DA4E 88566710
> --- Blank lines -----
> 3h
> --- Blank lines -----
> 
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> A3ABE6B398
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> A3ABE6B398
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> A3ABE6B398
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> A3ABE6B398
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1E18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1E18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1E18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1E18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1E18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1E18
> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B 8C0A1B18
> ASSERT [DxeCore]
> /uefi/MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c(3
> 00)
> : ((BOOLEAN)(0==1))
> 
> Here 387477C2-69C7-11D2-8E39-00A0C969723B GUID is
> gEfiSimpleTextOutProtocolGuid.
> 
> And there is no way to do source-level debug because FV image cannot be
> found in memory at the given location.
> As soon as I revert this commit
> (8ae5fc182941cf9ff7a222eb0a484088a0db8e2e), everything gets back to
> normal.
> Could you please explain me what I am doing wrong?
> 
> Thank you,
> Vladimir
> 
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard
> Biesheuvel
> Sent: Thursday, November 30, 2017 7:25 AM
> To: edk2-devel@lists.01.org
> Cc: leif.lindholm@linaro.org; Ard Biesheuvel
> Subject: [edk2] [PATCH 5/7] ArmPlatformPkg/MemoryInitPeiLib: don't reserve
> primary FV in memory
> 
> From: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> 
> Now that PrePi no longer exposes its internal code via special HOBs, we can
> remove the special handling of the primary FV, which needed to be reserved
> so that DXE core could call into the PE/COFF and LZMA libraries in the PrePi
> module.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Udit Kumar <udit.kumar@nxp.com>
> Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> [ardb: updated commit log]
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c | 69 --------------------
>  1 file changed, 69 deletions(-)
> 
> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> index 2feb11f21d5d..d03214b5df66 100644
> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> @@ -70,11 +70,7 @@ MemoryPeim (
>  {
>    ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable;
>    EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;
> -  UINT64                       ResourceLength;
>    EFI_PEI_HOB_POINTERS         NextHob;
> -  EFI_PHYSICAL_ADDRESS         FdTop;
> -  EFI_PHYSICAL_ADDRESS         SystemMemoryTop;
> -  EFI_PHYSICAL_ADDRESS         ResourceTop;
>    BOOLEAN                      Found;
> 
>    // Get Virtual Memory Map from the Platform Library @@ -121,71 +117,6
> @@ MemoryPeim (
>      );
>    }
> 
> -  //
> -  // Reserved the memory space occupied by the firmware volume
> -  //
> -
> -  SystemMemoryTop = (EFI_PHYSICAL_ADDRESS)PcdGet64
> (PcdSystemMemoryBase)
> + (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdSystemMemorySize);
> -  FdTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFdBaseAddress) +
> (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFdSize);
> -
> -  // EDK2 does not have the concept of boot firmware copied into DRAM. To
> avoid the DXE
> -  // core to overwrite this area we must mark the region with the attribute
> non-present
> -  if ((PcdGet64 (PcdFdBaseAddress) >= PcdGet64 (PcdSystemMemoryBase))
> && (FdTop <= SystemMemoryTop)) {
> -    Found = FALSE;
> -
> -    // Search for System Memory Hob that contains the firmware
> -    NextHob.Raw = GetHobList ();
> -    while ((NextHob.Raw = GetNextHob
> (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR,
> NextHob.Raw)) != NULL) {
> -      if ((NextHob.ResourceDescriptor->ResourceType ==
> EFI_RESOURCE_SYSTEM_MEMORY) &&
> -          (PcdGet64 (PcdFdBaseAddress) >=
> NextHob.ResourceDescriptor->PhysicalStart) &&
> -          (FdTop <= NextHob.ResourceDescriptor->PhysicalStart +
> NextHob.ResourceDescriptor->ResourceLength))
> -      {
> -        ResourceAttributes =
> NextHob.ResourceDescriptor->ResourceAttribute;
> -        ResourceLength = NextHob.ResourceDescriptor->ResourceLength;
> -        ResourceTop = NextHob.ResourceDescriptor->PhysicalStart +
> ResourceLength;
> -
> -        if (PcdGet64 (PcdFdBaseAddress) ==
> NextHob.ResourceDescriptor->PhysicalStart) {
> -          if (SystemMemoryTop == FdTop) {
> -            NextHob.ResourceDescriptor->ResourceAttribute =
> ResourceAttributes & ~EFI_RESOURCE_ATTRIBUTE_PRESENT;
> -          } else {
> -            // Create the System Memory HOB for the firmware with the
> non-present attribute
> -            BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
> -                                        ResourceAttributes &
> ~EFI_RESOURCE_ATTRIBUTE_PRESENT,
> -                                        PcdGet64 (PcdFdBaseAddress),
> -                                        PcdGet32 (PcdFdSize));
> -
> -            // Top of the FD is system memory available for UEFI
> -            NextHob.ResourceDescriptor->PhysicalStart +=
> PcdGet32(PcdFdSize);
> -            NextHob.ResourceDescriptor->ResourceLength -=
> PcdGet32(PcdFdSize);
> -          }
> -        } else {
> -          // Create the System Memory HOB for the firmware with the
> non-present attribute
> -          BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
> -                                      ResourceAttributes &
> ~EFI_RESOURCE_ATTRIBUTE_PRESENT,
> -                                      PcdGet64 (PcdFdBaseAddress),
> -                                      PcdGet32 (PcdFdSize));
> -
> -          // Update the HOB
> -          NextHob.ResourceDescriptor->ResourceLength = PcdGet64
> (PcdFdBaseAddress) - NextHob.ResourceDescriptor->PhysicalStart;
> -
> -          // If there is some memory available on the top of the FD then
> create a HOB
> -          if (FdTop < NextHob.ResourceDescriptor->PhysicalStart +
> ResourceLength) {
> -            // Create the System Memory HOB for the remaining region (top
> of the FD)
> -            BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
> -                                        ResourceAttributes,
> -                                        FdTop,
> -                                        ResourceTop - FdTop);
> -          }
> -        }
> -        Found = TRUE;
> -        break;
> -      }
> -      NextHob.Raw = GET_NEXT_HOB (NextHob);
> -    }
> -
> -    ASSERT(Found);
> -  }
> -
>    // Build Memory Allocation Hob
>    InitMmu (MemoryTable);
> 
> --
> 2.11.0
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.
> 01.org%2Fmailman%2Flistinfo%2Fedk2-
> devel&data=02%7C01%7Cudit.kumar%40nxp.com%7Ca75148c0714749bda72
> d08d54caaf73b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6364
> 99219600898902&sdata=nmhLohXfQGCJE3F%2FDan1YPZCcgbCZ6yyxcZsdZ71h
> P8%3D&reserved=0
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.
> 01.org%2Fmailman%2Flistinfo%2Fedk2-
> devel&data=02%7C01%7Cudit.kumar%40nxp.com%7Ca75148c0714749bda72
> d08d54caaf73b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6364
> 99219600898902&sdata=nmhLohXfQGCJE3F%2FDan1YPZCcgbCZ6yyxcZsdZ71h
> P8%3D&reserved=0


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2017-12-27  7:32 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-30 15:24 [PATCH 0/7] ArmPlatformPkg/PrePi: stop exposing internal code via HOBs Ard Biesheuvel
2017-11-30 15:24 ` [PATCH 1/7] EmbeddedPkg BeagleBoardPkg: move special HOB reuse libraries into platform Ard Biesheuvel
2017-11-30 15:24 ` [PATCH 2/7] BeagleBoardPkg: create private PrePi implementation Ard Biesheuvel
2017-11-30 15:24 ` [PATCH 3/7] BeagleBoardPkg: clone MemoryInitPeiLib Ard Biesheuvel
2017-11-30 15:24 ` [PATCH 4/7] ArmPlatformPkg/PrePi: don't expose PE/COFF and LZMA libraries via HOBs Ard Biesheuvel
2017-11-30 15:24 ` [PATCH 5/7] ArmPlatformPkg/MemoryInitPeiLib: don't reserve primary FV in memory Ard Biesheuvel
2017-12-26 21:52   ` Vladimir Olovyannikov
2017-12-26 23:06     ` Ard Biesheuvel
2017-12-27  1:58       ` Vladimir Olovyannikov
2017-12-27  2:01       ` Vladimir Olovyannikov
2017-12-27  7:37     ` Udit Kumar
2017-11-30 15:24 ` [PATCH 6/7] ArmPlatformPkg/PrePi; call all constructors by hand Ard Biesheuvel
2017-11-30 16:31   ` Leif Lindholm
2017-11-30 16:35     ` Ard Biesheuvel
2017-11-30 16:45       ` Leif Lindholm
2017-11-30 17:12         ` Ard Biesheuvel
2017-11-30 15:24 ` [PATCH 7/7] ArmPlatformPkg/PrePi: remove bogus IntelFrameworkModulePkg reference Ard Biesheuvel

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