ArmPlatformPkg/PrePi: avoid global variable write to mSystemMemoryEnd

The global variable mSystemMemoryEnd is initialized by PrePi only if
it has not been initialized by ArmPlatformPeiBootAction(). This allows
platforms executing under, e.g., ARM Trusted Firmware to dynamically
reserve a window at the top of memory that will be used by the secure
firmware.

However, PrePi is a SEC module, and writing to a global variable
violates the SEC constraints, since SEC and PEI may execute from NOR
flash.

So instead, initialize mSystemMemoryEnd statically. This will ensure
it holds the correct value for all implementations where the value
is not overridden, but still allows it to be overridden during the
call to ArmPlatformPeiBootAction().

Note that this patch also fixes a latent bug on 32-bit platforms where
a value of mSystemMemoryEnd exceeding 4 GB would be truncated to 32-bits
rather than limited to (4 GB - 1)

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Ryan Harkin <ryan.harkin@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

4 files changed, 14 insertions, 44 deletions

diff --git a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S
index d0530a8747..a81709d5d1 100644
--- a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S
+++ b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S
@@ -13,8 +13,6 @@
 

 #include <AsmMacroIoLibV8.h>

 

-ASM_GLOBAL ASM_PFX(mSystemMemoryEnd)

-

 ASM_FUNC(_ModuleEntryPoint)

   // Do early platform specific actions

   bl    ASM_PFX(ArmPlatformPeiBootAction)

@@ -31,16 +29,6 @@ _SetSVCMode:
 _SystemMemoryEndInit:

   ldr   x1, mSystemMemoryEnd

 

-  // Is mSystemMemoryEnd initialized?

-  cmp   x1, #0

-  bne   _SetupStackPosition

-

-  MOV64 (x1, FixedPcdGet64(PcdSystemMemoryBase) + FixedPcdGet64(PcdSystemMemorySize) - 1)

-

-  // Update the global variable

-  adr   x2, mSystemMemoryEnd

-  str   x1, [x2]

-

 _SetupStackPosition:

   // r1 = SystemMemoryTop

 

@@ -129,5 +117,3 @@ _PrepareArguments:
 

 _NeverReturn:

   b _NeverReturn

-

-ASM_PFX(mSystemMemoryEnd):    .8byte 0

diff --git a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S
index 39030da5f2..212cab62d4 100644
--- a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S
+++ b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S
@@ -15,8 +15,6 @@
 

 #include <Chipset/ArmV7.h>

 

-GCC_ASM_EXPORT(mSystemMemoryEnd)

-

 ASM_FUNC(_ModuleEntryPoint)

   // Do early platform specific actions

   bl    ASM_PFX(ArmPlatformPeiBootAction)

@@ -35,17 +33,11 @@ _SetSVCMode:
 // to install the stacks at the bottom of the Firmware Device (case the FD is located

 // at the top of the DRAM)

 _SystemMemoryEndInit:

-  ldr   r1, mSystemMemoryEnd

-

-  // Is mSystemMemoryEnd initialized?

-  cmp   r1, #0

-  bne   _SetupStackPosition

-

-  MOV32 (r1, FixedPcdGet32(PcdSystemMemoryBase) + FixedPcdGet32(PcdSystemMemorySize) - 1)

-

-  // Update the global variable

-  adr   r2, mSystemMemoryEnd

-  str   r1, [r2]

+  ADRL  (r1, mSystemMemoryEnd)

+  ldrd  r2, r3, [r1]

+  teq   r3, #0

+  moveq r1, r2

+  mvnne r1, #0

 

 _SetupStackPosition:

   // r1 = SystemMemoryTop

@@ -136,5 +128,3 @@ _PrepareArguments:
 

 _NeverReturn:

   b _NeverReturn

-

-ASM_PFX(mSystemMemoryEnd):  .8byte 0

diff --git a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.asm b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.asm
index 023339841f..1e9daf563b 100644
--- a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.asm
+++ b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.asm
@@ -21,15 +21,14 @@
   IMPORT  ArmReadMpidr

   IMPORT  ArmPlatformPeiBootAction

   IMPORT  ArmPlatformStackSet

+  IMPORT  mSystemMemoryEnd

 

   EXPORT  _ModuleEntryPoint

-  EXPORT  mSystemMemoryEnd

 

   PRESERVE8

   AREA    PrePiCoreEntryPoint, CODE, READONLY

 

 StartupAddr        DCD      CEntryPoint

-mSystemMemoryEnd   DCQ      0

 

 _ModuleEntryPoint

   // Do early platform specific actions

@@ -49,19 +48,11 @@ _SetSVCMode
 // to install the stacks at the bottom of the Firmware Device (case the FD is located

 // at the top of the DRAM)

 _SystemMemoryEndInit

-  ldr   r1, mSystemMemoryEnd

-

-  // Is mSystemMemoryEnd initialized?

-  cmp   r1, #0

-  bne   _SetupStackPosition

-

-  mov32 r1, FixedPcdGet32(PcdSystemMemoryBase)

-  mov32 r2, FixedPcdGet32(PcdSystemMemorySize)

-  sub   r2, r2, #1

-  add   r1, r1, r2

-  // Update the global variable

-  adr   r2, mSystemMemoryEnd

-  str   r1, [r2]

+  mov32 r1, mSystemMemoryEnd

+  ldrd  r2, r3, [r1]

+  teq   r3, #0

+  moveq r1, r2

+  mvnne r1, #0

 

 _SetupStackPosition

   // r1 = SystemMemoryTop

diff --git a/ArmPlatformPkg/PrePi/PrePi.c b/ArmPlatformPkg/PrePi/PrePi.c
index 36928c65a7..e548ccace0 100644
--- a/ArmPlatformPkg/PrePi/PrePi.c
+++ b/ArmPlatformPkg/PrePi/PrePi.c
@@ -32,6 +32,9 @@
 #define IS_XIP() (((UINT64)FixedPcdGet64 (PcdFdBaseAddress) > mSystemMemoryEnd) || \

                   ((FixedPcdGet64 (PcdFdBaseAddress) + FixedPcdGet32 (PcdFdSize)) < FixedPcdGet64 (PcdSystemMemoryBase)))

 

+UINT64 mSystemMemoryEnd = FixedPcdGet64(PcdSystemMemoryBase) +

+                          FixedPcdGet64(PcdSystemMemorySize) - 1;

+

 EFI_STATUS

 EFIAPI

 ExtractGuidedSectionLibConstructor (