diff mbox

[edk2,2/2] ArmPlatformPkg/PrePi: avoid global variable write to mSystemMemoryEnd

Message ID 1477299443-9324-2-git-send-email-ard.biesheuvel@linaro.org
State Accepted
Commit 4aae7419eeba36dfca48ecb59677a1ab27cffbb0
Headers show

Commit Message

Ard Biesheuvel Oct. 24, 2016, 8:57 a.m. UTC 
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>

---

Build tested only.

 ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S | 14 -------------
 ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S     | 20 +++++--------------
 ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.asm   | 21 ++++++--------------
 ArmPlatformPkg/PrePi/PrePi.c                    |  3 +++
 4 files changed, 14 insertions(+), 44 deletions(-)

-- 
2.7.4

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

Comments

Ryan Harkin Oct. 24, 2016, 12:05 p.m. UTC | #1 
On 24 October 2016 at 09:57, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> 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>


I tested this successfully on FVP Foundation & AEMv8 models, Juno
R0/1/2 and TC2.

Tested-by: Ryan Harkin <ryan.harkin@linaro.org>


> ---

>

> Build tested only.

>

>  ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S | 14 -------------

>  ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S     | 20 +++++--------------

>  ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.asm   | 21 ++++++--------------

>  ArmPlatformPkg/PrePi/PrePi.c                    |  3 +++

>  4 files changed, 14 insertions(+), 44 deletions(-)

>

> diff --git a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S

> index d0530a874726..a81709d5d12d 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 39030da5f2c3..212cab62d44b 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 023339841f75..1e9daf563bb6 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 36928c65a73b..e548ccace097 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 (

> --

> 2.7.4

>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Oct. 24, 2016, 12:21 p.m. UTC | #2 
On 24 October 2016 at 13:05, Ryan Harkin <ryan.harkin@linaro.org> wrote:
> On 24 October 2016 at 09:57, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

>> 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>

>

> I tested this successfully on FVP Foundation & AEMv8 models, Juno

> R0/1/2 and TC2.

>

> Tested-by: Ryan Harkin <ryan.harkin@linaro.org>

>


Many thanks!
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Leif Lindholm Oct. 24, 2016, 2:42 p.m. UTC | #3 
On Mon, Oct 24, 2016 at 09:57:23AM +0100, Ard Biesheuvel wrote:
> 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.


So, I agree this is probably why this variable was added to begin
with, but I don't think it necessarily solves a real problem these
days. (There is no guarantee ARM-TF will live at top of RAM to begin
with.)

But the fix for that probably includes killing of SystemMemoryBase and
SystemMemorySize at the same time...

> 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>


Since this is already a clear improvement:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>


> ---

> 

> Build tested only.

> 

>  ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S | 14 -------------

>  ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S     | 20 +++++--------------

>  ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.asm   | 21 ++++++--------------

>  ArmPlatformPkg/PrePi/PrePi.c                    |  3 +++

>  4 files changed, 14 insertions(+), 44 deletions(-)

> 

> diff --git a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S

> index d0530a874726..a81709d5d12d 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 39030da5f2c3..212cab62d44b 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 023339841f75..1e9daf563bb6 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 36928c65a73b..e548ccace097 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 (

> -- 

> 2.7.4

> 

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

Patch

diff --git a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S
index d0530a874726..a81709d5d12d 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 39030da5f2c3..212cab62d44b 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 023339841f75..1e9daf563bb6 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 36928c65a73b..e548ccace097 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 (