From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: edk2-devel@lists.01.org
Cc: leif.lindholm@linaro.org, lersek@redhat.com, heyi.guo@Linaro.org,
ryan.harkin@linaro.org,
Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: [PATCH 2/2] ArmPlatformPkg/PrePi: avoid global variable write to mSystemMemoryEnd
Date: Mon, 24 Oct 2016 09:57:23 +0100 [thread overview]
Message-ID: <1477299443-9324-2-git-send-email-ard.biesheuvel@linaro.org> (raw)
In-Reply-To: <1477299443-9324-1-git-send-email-ard.biesheuvel@linaro.org>
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(-)
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
next prev parent reply other threads:[~2016-10-24 8:57 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-24 8:57 [PATCH 1/2] ArmVirtPkg/PrePi: remove mSystemMemoryEnd Ard Biesheuvel
2016-10-24 8:57 ` Ard Biesheuvel [this message]
2016-10-24 12:05 ` [PATCH 2/2] ArmPlatformPkg/PrePi: avoid global variable write to mSystemMemoryEnd Ryan Harkin
2016-10-24 12:21 ` Ard Biesheuvel
2016-10-24 14:42 ` Leif Lindholm
2016-10-24 11:29 ` [PATCH 1/2] ArmVirtPkg/PrePi: remove mSystemMemoryEnd Laszlo Ersek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1477299443-9324-2-git-send-email-ard.biesheuvel@linaro.org \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox