On 22 October 2016 at 09:09, Ard Biesheuvel ard.biesheuvel@linaro.org wrote:
On 22 October 2016 at 04:21, Heyi Guo heyi.guo@linaro.org wrote:
Hi folks,
We are still using PrePi on some of our platforms and the code is still running on Flash at this stage. We found there is global data write in ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S. The change was introduced by this commit: 5dbacdb21b59748e885c2eccae370b81271ab795:
+mSystemMemoryEnd: .8byte 0^M
ASM_PFX(_ModuleEntryPoint): // Do early platform specific actions @@ -40,12 +42,23 @@ _SetSVCMode: // 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) -_SetupStackPosition:
- // Compute Top of System Memory
- LoadConstantToReg (FixedPcdGet64 (PcdSystemMemoryBase), x1)
- LoadConstantToReg (FixedPcdGet64 (PcdSystemMemorySize), x2)
+_SystemMemoryEndInit:^M
- ldr x1, mSystemMemoryEnd^M
+^M
- // Is mSystemMemoryEnd initialized?^M
- cmp x1, #0^M
- bne _SetupStackPosition^M
+^M
- LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryBase), x1)^M
- LoadConstantToReg (FixedPcdGet32(PcdSystemMemorySize), x2)^M sub x2, x2, #1
- add x1, x1, x2 // x1 = SystemMemoryTop = PcdSystemMemoryBase +
PcdSystemMemorySize
- add x1, x1, x2^M
- // Update the global variable^M
- adr x2, mSystemMemoryEnd^M
- str x1, [x2]^M
I think direct write to flash should be forbidden. This change may work well for platforms which use ATF and load PrePi into memory, but not for other platforms.
Please let me know your comments about this.
You are right. This should be removed. I think it was added for ATF on Juno, but obviously, this is not the right way to do it, given that SEC and PEIM modules may run from NOR.
Does this work for you?
--- a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S +++ b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S @@ -28,18 +28,6 @@ _SetSVCMode: // 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: - 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 @@ -130,4 +118,5 @@ _PrepareArguments: _NeverReturn: b _NeverReturn
-ASM_PFX(mSystemMemoryEnd): .8byte 0 +ASM_PFX(mSystemMemoryEnd): + .8byte FixedPcdGet64(PcdSystemMemoryBase) + FixedPcdGet64(PcdSystemMemorySize) - 1