| Message ID | 1470842282-8415-23-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | Accepted |
| Commit | d2fa09a1348738a770f7a7dcb1ec020e6b3a3d0c |
| Headers | show |
On Wed, Aug 10, 2016 at 05:17:58PM +0200, Ard Biesheuvel wrote: > Annotate functions with ASM_FUNC() so that they are emitted into > separate sections. Also replacing LoadConstantToReg. Add that to commit message and: Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S | 49 ++++++------------- > ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S | 50 ++++++-------------- > 2 files changed, 29 insertions(+), 70 deletions(-) > > diff --git a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S > index 9538c70a237c..d0530a874726 100644 > --- a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S > +++ b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S > @@ -12,24 +12,10 @@ > // > > #include <AsmMacroIoLibV8.h> > -#include <Base.h> > -#include <Library/PcdLib.h> > -#include <AutoGen.h> > - > -.text > -.align 3 > - > -GCC_ASM_IMPORT(ArmPlatformIsPrimaryCore) > -GCC_ASM_IMPORT(ArmReadMpidr) > -GCC_ASM_IMPORT(ArmPlatformPeiBootAction) > -GCC_ASM_IMPORT(ArmPlatformStackSet) > -GCC_ASM_EXPORT(_ModuleEntryPoint) > -ASM_GLOBAL ASM_PFX(mSystemMemoryEnd) > > -StartupAddr: .8byte ASM_PFX(CEntryPoint) > -ASM_PFX(mSystemMemoryEnd): .8byte 0 > +ASM_GLOBAL ASM_PFX(mSystemMemoryEnd) > > -ASM_PFX(_ModuleEntryPoint): > +ASM_FUNC(_ModuleEntryPoint) > // Do early platform specific actions > bl ASM_PFX(ArmPlatformPeiBootAction) > > @@ -49,10 +35,8 @@ _SystemMemoryEndInit: > cmp x1, #0 > bne _SetupStackPosition > > - LoadConstantToReg (FixedPcdGet64(PcdSystemMemoryBase), x1) > - LoadConstantToReg (FixedPcdGet64(PcdSystemMemorySize), x2) > - sub x2, x2, #1 > - add x1, x1, x2 > + MOV64 (x1, FixedPcdGet64(PcdSystemMemoryBase) + FixedPcdGet64(PcdSystemMemorySize) - 1) > + > // Update the global variable > adr x2, mSystemMemoryEnd > str x1, [x2] > @@ -61,13 +45,13 @@ _SetupStackPosition: > // r1 = SystemMemoryTop > > // Calculate Top of the Firmware Device > - LoadConstantToReg (FixedPcdGet64(PcdFdBaseAddress), x2) > - LoadConstantToReg (FixedPcdGet32(PcdFdSize), x3) > + MOV64 (x2, FixedPcdGet64(PcdFdBaseAddress)) > + MOV32 (x3, FixedPcdGet32(PcdFdSize) - 1) > sub x3, x3, #1 > add x3, x3, x2 // x3 = FdTop = PcdFdBaseAddress + PcdFdSize > > // UEFI Memory Size (stacks are allocated in this region) > - LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryUefiRegionSize), x4) > + MOV32 (x4, FixedPcdGet32(PcdSystemMemoryUefiRegionSize)) > > // > // Reserve the memory for the UEFI region (contain stacks on its top) > @@ -98,9 +82,7 @@ _SetupAlignedStack: > _SetupOverflowStack: > // Case memory at the top of the address space. Ensure the top of the stack is EFI_PAGE_SIZE > // aligned (4KB) > - LoadConstantToReg (EFI_PAGE_MASK, x11) > - and x11, x11, x1 > - sub x1, x1, x11 > + and x1, x1, ~EFI_PAGE_MASK > > _GetBaseUefiMemory: > // Calculate the Base of the UEFI Memory > @@ -109,22 +91,19 @@ _GetBaseUefiMemory: > _GetStackBase: > // r1 = The top of the Mpcore Stacks > // Stack for the primary core = PrimaryCoreStack > - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), x2) > + MOV32 (x2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) > sub x12, x1, x2 > > // Stack for the secondary core = Number of Cores - 1 > - LoadConstantToReg (FixedPcdGet32(PcdCoreCount), x0) > - sub x0, x0, #1 > - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), x1) > - mul x1, x1, x0 > + MOV32 (x1, (FixedPcdGet32(PcdCoreCount) - 1) * FixedPcdGet32(PcdCPUCoreSecondaryStackSize)) > sub x12, x12, x1 > > // x12 = The base of the MpCore Stacks (primary stack & secondary stacks) > mov x0, x12 > mov x1, x10 > //ArmPlatformStackSet(StackBase, MpId, PrimaryStackSize, SecondaryStackSize) > - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), x2) > - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), x3) > + MOV32 (x2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) > + MOV32 (x3, FixedPcdGet32(PcdCPUCoreSecondaryStackSize)) > bl ASM_PFX(ArmPlatformStackSet) > > // Is it the Primary Core ? > @@ -140,7 +119,7 @@ _PrepareArguments: > > // Move sec startup address into a data register > // Ensure we're jumping to FV version of the code (not boot remapped alias) > - ldr x4, StartupAddr > + ldr x4, =ASM_PFX(CEntryPoint) > > // Jump to PrePiCore C code > // x0 = MpId > @@ -150,3 +129,5 @@ _PrepareArguments: > > _NeverReturn: > b _NeverReturn > + > +ASM_PFX(mSystemMemoryEnd): .8byte 0 > diff --git a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S > index 1311efc5cb2c..b7127ce9fb4c 100644 > --- a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S > +++ b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S > @@ -12,28 +12,12 @@ > // > > #include <AsmMacroIoLib.h> > -#include <Base.h> > -#include <Library/PcdLib.h> > -#include <AutoGen.h> > > #include <Chipset/ArmV7.h> > > -.text > -.align 3 > - > -GCC_ASM_IMPORT(CEntryPoint) > -GCC_ASM_IMPORT(ArmPlatformIsPrimaryCore) > -GCC_ASM_IMPORT(ArmReadMpidr) > -GCC_ASM_IMPORT(ArmPlatformPeiBootAction) > -GCC_ASM_IMPORT(ArmPlatformStackSet) > -GCC_ASM_EXPORT(_ModuleEntryPoint) > GCC_ASM_EXPORT(mSystemMemoryEnd) > > -StartupAddr: .word CEntryPoint > -mSystemMemoryEnd: .8byte 0 > - > - > -ASM_PFX(_ModuleEntryPoint): > +ASM_FUNC(_ModuleEntryPoint) > // Do early platform specific actions > bl ASM_PFX(ArmPlatformPeiBootAction) > > @@ -57,10 +41,8 @@ _SystemMemoryEndInit: > cmp r1, #0 > bne _SetupStackPosition > > - LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryBase), r1) > - LoadConstantToReg (FixedPcdGet32(PcdSystemMemorySize), r2) > - sub r2, r2, #1 > - add r1, r1, r2 > + MOV32 (r1, FixedPcdGet32(PcdSystemMemoryBase) + FixedPcdGet32(PcdSystemMemorySize) - 1) > + > // Update the global variable > adr r2, mSystemMemoryEnd > str r1, [r2] > @@ -69,13 +51,12 @@ _SetupStackPosition: > // r1 = SystemMemoryTop > > // Calculate Top of the Firmware Device > - LoadConstantToReg (FixedPcdGet32(PcdFdBaseAddress), r2) > - LoadConstantToReg (FixedPcdGet32(PcdFdSize), r3) > - sub r3, r3, #1 > + 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) > - LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryUefiRegionSize), r4) > + MOV32 (r4, FixedPcdGet32(PcdSystemMemoryUefiRegionSize)) > > // > // Reserve the memory for the UEFI region (contain stacks on its top) > @@ -106,9 +87,8 @@ _SetupAlignedStack: > _SetupOverflowStack: > // Case memory at the top of the address space. Ensure the top of the stack is EFI_PAGE_SIZE > // aligned (4KB) > - LoadConstantToReg (EFI_PAGE_MASK, r9) > - and r9, r9, r1 > - sub r1, r1, r9 > + MOV32 (r9, ~EFI_PAGE_MASK & 0xFFFFFFFF) > + and r1, r1, r9 > > _GetBaseUefiMemory: > // Calculate the Base of the UEFI Memory > @@ -117,22 +97,19 @@ _GetBaseUefiMemory: > _GetStackBase: > // r1 = The top of the Mpcore Stacks > // Stack for the primary core = PrimaryCoreStack > - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), r2) > + MOV32 (r2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) > sub r10, r1, r2 > > // Stack for the secondary core = Number of Cores - 1 > - LoadConstantToReg (FixedPcdGet32(PcdCoreCount), r0) > - sub r0, r0, #1 > - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), r1) > - mul r1, r1, r0 > + MOV32 (r0, (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) > - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), r2) > - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), r3) > + MOV32 (r2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) > + MOV32 (r3, FixedPcdGet32(PcdCPUCoreSecondaryStackSize)) > bl ASM_PFX(ArmPlatformStackSet) > > // Is it the Primary Core ? > @@ -149,7 +126,7 @@ _PrepareArguments: > > // 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 > + ldr r4, =ASM_PFX(CEntryPoint) > > // Jump to PrePiCore C code > // r0 = MpId > @@ -160,3 +137,4 @@ _PrepareArguments: > _NeverReturn: > b _NeverReturn > > +ASM_PFX(mSystemMemoryEnd): .8byte 0 > -- > 2.7.4 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 31 August 2016 at 05:33, Michael Zimmermann <sigmaepsilon92@gmail.com> wrote: > reviewed should mean tested ;) > Apologies for the breakage. As it turns out, ArmVirtQemuKernel does work even with this bug, probably due to the fact that its PcdCoreCount == 1 (or simply that it uses the UniCore flavor rather than the MPCore flavor of PrePi) In any case, thanks for tracking this down, and for proposing a fix. > took me some time to find out why my system currently doesn't boot anymore > but here's the fix for this commit: > > diff --git a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S > b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S > index b7127ce..39030da 100644 > --- a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S > +++ b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S > @@ -101,7 +101,7 @@ _GetStackBase: > sub r10, r1, r2 > > // Stack for the secondary core = Number of Cores - 1 > - MOV32 (r0, (FixedPcdGet32(PcdCoreCount) - 1) * > FixedPcdGet32(PcdCPUCoreSecondaryStackSize)) > + MOV32 (r1, (FixedPcdGet32(PcdCoreCount) - 1) * > FixedPcdGet32(PcdCPUCoreSecondaryStackSize))^M > sub r10, r10, r1 > > // r10 = The base of the MpCore Stacks (primary stack & secondary stacks) > > On Thu, Aug 11, 2016 at 10:38 AM, Leif Lindholm <leif.lindholm@linaro.org> > wrote: >> >> On Wed, Aug 10, 2016 at 05:17:58PM +0200, Ard Biesheuvel wrote: >> > Annotate functions with ASM_FUNC() so that they are emitted into >> > separate sections. >> >> Also replacing LoadConstantToReg. Add that to commit message and: >> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> >> >> > Contributed-under: TianoCore Contribution Agreement 1.0 >> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> > --- >> > ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S | 49 >> > ++++++------------- >> > ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S | 50 >> > ++++++-------------- >> > 2 files changed, 29 insertions(+), 70 deletions(-) >> > >> > diff --git a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S >> > b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S >> > index 9538c70a237c..d0530a874726 100644 >> > --- a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S >> > +++ b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S >> > @@ -12,24 +12,10 @@ >> > // >> > >> > #include <AsmMacroIoLibV8.h> >> > -#include <Base.h> >> > -#include <Library/PcdLib.h> >> > -#include <AutoGen.h> >> > - >> > -.text >> > -.align 3 >> > - >> > -GCC_ASM_IMPORT(ArmPlatformIsPrimaryCore) >> > -GCC_ASM_IMPORT(ArmReadMpidr) >> > -GCC_ASM_IMPORT(ArmPlatformPeiBootAction) >> > -GCC_ASM_IMPORT(ArmPlatformStackSet) >> > -GCC_ASM_EXPORT(_ModuleEntryPoint) >> > -ASM_GLOBAL ASM_PFX(mSystemMemoryEnd) >> > >> > -StartupAddr: .8byte ASM_PFX(CEntryPoint) >> > -ASM_PFX(mSystemMemoryEnd): .8byte 0 >> > +ASM_GLOBAL ASM_PFX(mSystemMemoryEnd) >> > >> > -ASM_PFX(_ModuleEntryPoint): >> > +ASM_FUNC(_ModuleEntryPoint) >> > // Do early platform specific actions >> > bl ASM_PFX(ArmPlatformPeiBootAction) >> > >> > @@ -49,10 +35,8 @@ _SystemMemoryEndInit: >> > cmp x1, #0 >> > bne _SetupStackPosition >> > >> > - LoadConstantToReg (FixedPcdGet64(PcdSystemMemoryBase), x1) >> > - LoadConstantToReg (FixedPcdGet64(PcdSystemMemorySize), x2) >> > - sub x2, x2, #1 >> > - add x1, x1, x2 >> > + MOV64 (x1, FixedPcdGet64(PcdSystemMemoryBase) + >> > FixedPcdGet64(PcdSystemMemorySize) - 1) >> > + >> > // Update the global variable >> > adr x2, mSystemMemoryEnd >> > str x1, [x2] >> > @@ -61,13 +45,13 @@ _SetupStackPosition: >> > // r1 = SystemMemoryTop >> > >> > // Calculate Top of the Firmware Device >> > - LoadConstantToReg (FixedPcdGet64(PcdFdBaseAddress), x2) >> > - LoadConstantToReg (FixedPcdGet32(PcdFdSize), x3) >> > + MOV64 (x2, FixedPcdGet64(PcdFdBaseAddress)) >> > + MOV32 (x3, FixedPcdGet32(PcdFdSize) - 1) >> > sub x3, x3, #1 >> > add x3, x3, x2 // x3 = FdTop = PcdFdBaseAddress + PcdFdSize >> > >> > // UEFI Memory Size (stacks are allocated in this region) >> > - LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryUefiRegionSize), x4) >> > + MOV32 (x4, FixedPcdGet32(PcdSystemMemoryUefiRegionSize)) >> > >> > // >> > // Reserve the memory for the UEFI region (contain stacks on its top) >> > @@ -98,9 +82,7 @@ _SetupAlignedStack: >> > _SetupOverflowStack: >> > // Case memory at the top of the address space. Ensure the top of the >> > stack is EFI_PAGE_SIZE >> > // aligned (4KB) >> > - LoadConstantToReg (EFI_PAGE_MASK, x11) >> > - and x11, x11, x1 >> > - sub x1, x1, x11 >> > + and x1, x1, ~EFI_PAGE_MASK >> > >> > _GetBaseUefiMemory: >> > // Calculate the Base of the UEFI Memory >> > @@ -109,22 +91,19 @@ _GetBaseUefiMemory: >> > _GetStackBase: >> > // r1 = The top of the Mpcore Stacks >> > // Stack for the primary core = PrimaryCoreStack >> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), x2) >> > + MOV32 (x2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) >> > sub x12, x1, x2 >> > >> > // Stack for the secondary core = Number of Cores - 1 >> > - LoadConstantToReg (FixedPcdGet32(PcdCoreCount), x0) >> > - sub x0, x0, #1 >> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), x1) >> > - mul x1, x1, x0 >> > + MOV32 (x1, (FixedPcdGet32(PcdCoreCount) - 1) * >> > FixedPcdGet32(PcdCPUCoreSecondaryStackSize)) >> > sub x12, x12, x1 >> > >> > // x12 = The base of the MpCore Stacks (primary stack & secondary >> > stacks) >> > mov x0, x12 >> > mov x1, x10 >> > //ArmPlatformStackSet(StackBase, MpId, PrimaryStackSize, >> > SecondaryStackSize) >> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), x2) >> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), x3) >> > + MOV32 (x2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) >> > + MOV32 (x3, FixedPcdGet32(PcdCPUCoreSecondaryStackSize)) >> > bl ASM_PFX(ArmPlatformStackSet) >> > >> > // Is it the Primary Core ? >> > @@ -140,7 +119,7 @@ _PrepareArguments: >> > >> > // Move sec startup address into a data register >> > // Ensure we're jumping to FV version of the code (not boot remapped >> > alias) >> > - ldr x4, StartupAddr >> > + ldr x4, =ASM_PFX(CEntryPoint) >> > >> > // Jump to PrePiCore C code >> > // x0 = MpId >> > @@ -150,3 +129,5 @@ _PrepareArguments: >> > >> > _NeverReturn: >> > b _NeverReturn >> > + >> > +ASM_PFX(mSystemMemoryEnd): .8byte 0 >> > diff --git a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >> > b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >> > index 1311efc5cb2c..b7127ce9fb4c 100644 >> > --- a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >> > +++ b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >> > @@ -12,28 +12,12 @@ >> > // >> > >> > #include <AsmMacroIoLib.h> >> > -#include <Base.h> >> > -#include <Library/PcdLib.h> >> > -#include <AutoGen.h> >> > >> > #include <Chipset/ArmV7.h> >> > >> > -.text >> > -.align 3 >> > - >> > -GCC_ASM_IMPORT(CEntryPoint) >> > -GCC_ASM_IMPORT(ArmPlatformIsPrimaryCore) >> > -GCC_ASM_IMPORT(ArmReadMpidr) >> > -GCC_ASM_IMPORT(ArmPlatformPeiBootAction) >> > -GCC_ASM_IMPORT(ArmPlatformStackSet) >> > -GCC_ASM_EXPORT(_ModuleEntryPoint) >> > GCC_ASM_EXPORT(mSystemMemoryEnd) >> > >> > -StartupAddr: .word CEntryPoint >> > -mSystemMemoryEnd: .8byte 0 >> > - >> > - >> > -ASM_PFX(_ModuleEntryPoint): >> > +ASM_FUNC(_ModuleEntryPoint) >> > // Do early platform specific actions >> > bl ASM_PFX(ArmPlatformPeiBootAction) >> > >> > @@ -57,10 +41,8 @@ _SystemMemoryEndInit: >> > cmp r1, #0 >> > bne _SetupStackPosition >> > >> > - LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryBase), r1) >> > - LoadConstantToReg (FixedPcdGet32(PcdSystemMemorySize), r2) >> > - sub r2, r2, #1 >> > - add r1, r1, r2 >> > + MOV32 (r1, FixedPcdGet32(PcdSystemMemoryBase) + >> > FixedPcdGet32(PcdSystemMemorySize) - 1) >> > + >> > // Update the global variable >> > adr r2, mSystemMemoryEnd >> > str r1, [r2] >> > @@ -69,13 +51,12 @@ _SetupStackPosition: >> > // r1 = SystemMemoryTop >> > >> > // Calculate Top of the Firmware Device >> > - LoadConstantToReg (FixedPcdGet32(PcdFdBaseAddress), r2) >> > - LoadConstantToReg (FixedPcdGet32(PcdFdSize), r3) >> > - sub r3, r3, #1 >> > + 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) >> > - LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryUefiRegionSize), r4) >> > + MOV32 (r4, FixedPcdGet32(PcdSystemMemoryUefiRegionSize)) >> > >> > // >> > // Reserve the memory for the UEFI region (contain stacks on its top) >> > @@ -106,9 +87,8 @@ _SetupAlignedStack: >> > _SetupOverflowStack: >> > // Case memory at the top of the address space. Ensure the top of the >> > stack is EFI_PAGE_SIZE >> > // aligned (4KB) >> > - LoadConstantToReg (EFI_PAGE_MASK, r9) >> > - and r9, r9, r1 >> > - sub r1, r1, r9 >> > + MOV32 (r9, ~EFI_PAGE_MASK & 0xFFFFFFFF) >> > + and r1, r1, r9 >> > >> > _GetBaseUefiMemory: >> > // Calculate the Base of the UEFI Memory >> > @@ -117,22 +97,19 @@ _GetBaseUefiMemory: >> > _GetStackBase: >> > // r1 = The top of the Mpcore Stacks >> > // Stack for the primary core = PrimaryCoreStack >> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), r2) >> > + MOV32 (r2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) >> > sub r10, r1, r2 >> > >> > // Stack for the secondary core = Number of Cores - 1 >> > - LoadConstantToReg (FixedPcdGet32(PcdCoreCount), r0) >> > - sub r0, r0, #1 >> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), r1) >> > - mul r1, r1, r0 >> > + MOV32 (r0, (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) >> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), r2) >> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), r3) >> > + MOV32 (r2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) >> > + MOV32 (r3, FixedPcdGet32(PcdCPUCoreSecondaryStackSize)) >> > bl ASM_PFX(ArmPlatformStackSet) >> > >> > // Is it the Primary Core ? >> > @@ -149,7 +126,7 @@ _PrepareArguments: >> > >> > // 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 >> > + ldr r4, =ASM_PFX(CEntryPoint) >> > >> > // Jump to PrePiCore C code >> > // r0 = MpId >> > @@ -160,3 +137,4 @@ _PrepareArguments: >> > _NeverReturn: >> > b _NeverReturn >> > >> > +ASM_PFX(mSystemMemoryEnd): .8byte 0 >> > -- >> > 2.7.4 >> > >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel > > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 31 August 2016 at 10:43, Michael Zimmermann <sigmaepsilon92@gmail.com> wrote: > ArmVirtQemuKernel uses ArmVirtPkg/PrePi/Arm/ModuleEntryPoint.S which is not > affected by this bug. > Ah, of course. It does highlight why having so many copies of essentially the same code is a bad idea, but it is a difficult pattern to get rid of in Tianocore. > as it turns out, only BeagleBoardPkg uses the generic PrePi code. > Thanks, Ard. > On Wed, Aug 31, 2016 at 11:14 AM, Ard Biesheuvel <ard.biesheuvel@linaro.org> > wrote: >> >> On 31 August 2016 at 05:33, Michael Zimmermann <sigmaepsilon92@gmail.com> >> wrote: >> > reviewed should mean tested ;) >> > >> >> Apologies for the breakage. As it turns out, ArmVirtQemuKernel does >> work even with this bug, probably due to the fact that its >> PcdCoreCount == 1 (or simply that it uses the UniCore flavor rather >> than the MPCore flavor of PrePi) >> >> In any case, thanks for tracking this down, and for proposing a fix. >> >> > took me some time to find out why my system currently doesn't boot >> > anymore >> > but here's the fix for this commit: >> > >> > diff --git a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >> > b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >> > index b7127ce..39030da 100644 >> > --- a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >> > +++ b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >> > @@ -101,7 +101,7 @@ _GetStackBase: >> > sub r10, r1, r2 >> > >> > // Stack for the secondary core = Number of Cores - 1 >> > - MOV32 (r0, (FixedPcdGet32(PcdCoreCount) - 1) * >> > FixedPcdGet32(PcdCPUCoreSecondaryStackSize)) >> > + MOV32 (r1, (FixedPcdGet32(PcdCoreCount) - 1) * >> > FixedPcdGet32(PcdCPUCoreSecondaryStackSize))^M >> > sub r10, r10, r1 >> > >> > // r10 = The base of the MpCore Stacks (primary stack & secondary >> > stacks) >> > >> > On Thu, Aug 11, 2016 at 10:38 AM, Leif Lindholm >> > <leif.lindholm@linaro.org> >> > wrote: >> >> >> >> On Wed, Aug 10, 2016 at 05:17:58PM +0200, Ard Biesheuvel wrote: >> >> > Annotate functions with ASM_FUNC() so that they are emitted into >> >> > separate sections. >> >> >> >> Also replacing LoadConstantToReg. Add that to commit message and: >> >> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> >> >> >> >> > Contributed-under: TianoCore Contribution Agreement 1.0 >> >> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> >> > --- >> >> > ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S | 49 >> >> > ++++++------------- >> >> > ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S | 50 >> >> > ++++++-------------- >> >> > 2 files changed, 29 insertions(+), 70 deletions(-) >> >> > >> >> > diff --git a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S >> >> > b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S >> >> > index 9538c70a237c..d0530a874726 100644 >> >> > --- a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S >> >> > +++ b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S >> >> > @@ -12,24 +12,10 @@ >> >> > // >> >> > >> >> > #include <AsmMacroIoLibV8.h> >> >> > -#include <Base.h> >> >> > -#include <Library/PcdLib.h> >> >> > -#include <AutoGen.h> >> >> > - >> >> > -.text >> >> > -.align 3 >> >> > - >> >> > -GCC_ASM_IMPORT(ArmPlatformIsPrimaryCore) >> >> > -GCC_ASM_IMPORT(ArmReadMpidr) >> >> > -GCC_ASM_IMPORT(ArmPlatformPeiBootAction) >> >> > -GCC_ASM_IMPORT(ArmPlatformStackSet) >> >> > -GCC_ASM_EXPORT(_ModuleEntryPoint) >> >> > -ASM_GLOBAL ASM_PFX(mSystemMemoryEnd) >> >> > >> >> > -StartupAddr: .8byte ASM_PFX(CEntryPoint) >> >> > -ASM_PFX(mSystemMemoryEnd): .8byte 0 >> >> > +ASM_GLOBAL ASM_PFX(mSystemMemoryEnd) >> >> > >> >> > -ASM_PFX(_ModuleEntryPoint): >> >> > +ASM_FUNC(_ModuleEntryPoint) >> >> > // Do early platform specific actions >> >> > bl ASM_PFX(ArmPlatformPeiBootAction) >> >> > >> >> > @@ -49,10 +35,8 @@ _SystemMemoryEndInit: >> >> > cmp x1, #0 >> >> > bne _SetupStackPosition >> >> > >> >> > - LoadConstantToReg (FixedPcdGet64(PcdSystemMemoryBase), x1) >> >> > - LoadConstantToReg (FixedPcdGet64(PcdSystemMemorySize), x2) >> >> > - sub x2, x2, #1 >> >> > - add x1, x1, x2 >> >> > + MOV64 (x1, FixedPcdGet64(PcdSystemMemoryBase) + >> >> > FixedPcdGet64(PcdSystemMemorySize) - 1) >> >> > + >> >> > // Update the global variable >> >> > adr x2, mSystemMemoryEnd >> >> > str x1, [x2] >> >> > @@ -61,13 +45,13 @@ _SetupStackPosition: >> >> > // r1 = SystemMemoryTop >> >> > >> >> > // Calculate Top of the Firmware Device >> >> > - LoadConstantToReg (FixedPcdGet64(PcdFdBaseAddress), x2) >> >> > - LoadConstantToReg (FixedPcdGet32(PcdFdSize), x3) >> >> > + MOV64 (x2, FixedPcdGet64(PcdFdBaseAddress)) >> >> > + MOV32 (x3, FixedPcdGet32(PcdFdSize) - 1) >> >> > sub x3, x3, #1 >> >> > add x3, x3, x2 // x3 = FdTop = PcdFdBaseAddress + PcdFdSize >> >> > >> >> > // UEFI Memory Size (stacks are allocated in this region) >> >> > - LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryUefiRegionSize), >> >> > x4) >> >> > + MOV32 (x4, FixedPcdGet32(PcdSystemMemoryUefiRegionSize)) >> >> > >> >> > // >> >> > // Reserve the memory for the UEFI region (contain stacks on its >> >> > top) >> >> > @@ -98,9 +82,7 @@ _SetupAlignedStack: >> >> > _SetupOverflowStack: >> >> > // Case memory at the top of the address space. Ensure the top of >> >> > the >> >> > stack is EFI_PAGE_SIZE >> >> > // aligned (4KB) >> >> > - LoadConstantToReg (EFI_PAGE_MASK, x11) >> >> > - and x11, x11, x1 >> >> > - sub x1, x1, x11 >> >> > + and x1, x1, ~EFI_PAGE_MASK >> >> > >> >> > _GetBaseUefiMemory: >> >> > // Calculate the Base of the UEFI Memory >> >> > @@ -109,22 +91,19 @@ _GetBaseUefiMemory: >> >> > _GetStackBase: >> >> > // r1 = The top of the Mpcore Stacks >> >> > // Stack for the primary core = PrimaryCoreStack >> >> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), x2) >> >> > + MOV32 (x2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) >> >> > sub x12, x1, x2 >> >> > >> >> > // Stack for the secondary core = Number of Cores - 1 >> >> > - LoadConstantToReg (FixedPcdGet32(PcdCoreCount), x0) >> >> > - sub x0, x0, #1 >> >> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), >> >> > x1) >> >> > - mul x1, x1, x0 >> >> > + MOV32 (x1, (FixedPcdGet32(PcdCoreCount) - 1) * >> >> > FixedPcdGet32(PcdCPUCoreSecondaryStackSize)) >> >> > sub x12, x12, x1 >> >> > >> >> > // x12 = The base of the MpCore Stacks (primary stack & secondary >> >> > stacks) >> >> > mov x0, x12 >> >> > mov x1, x10 >> >> > //ArmPlatformStackSet(StackBase, MpId, PrimaryStackSize, >> >> > SecondaryStackSize) >> >> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), x2) >> >> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), >> >> > x3) >> >> > + MOV32 (x2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) >> >> > + MOV32 (x3, FixedPcdGet32(PcdCPUCoreSecondaryStackSize)) >> >> > bl ASM_PFX(ArmPlatformStackSet) >> >> > >> >> > // Is it the Primary Core ? >> >> > @@ -140,7 +119,7 @@ _PrepareArguments: >> >> > >> >> > // Move sec startup address into a data register >> >> > // Ensure we're jumping to FV version of the code (not boot >> >> > remapped >> >> > alias) >> >> > - ldr x4, StartupAddr >> >> > + ldr x4, =ASM_PFX(CEntryPoint) >> >> > >> >> > // Jump to PrePiCore C code >> >> > // x0 = MpId >> >> > @@ -150,3 +129,5 @@ _PrepareArguments: >> >> > >> >> > _NeverReturn: >> >> > b _NeverReturn >> >> > + >> >> > +ASM_PFX(mSystemMemoryEnd): .8byte 0 >> >> > diff --git a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >> >> > b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >> >> > index 1311efc5cb2c..b7127ce9fb4c 100644 >> >> > --- a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >> >> > +++ b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >> >> > @@ -12,28 +12,12 @@ >> >> > // >> >> > >> >> > #include <AsmMacroIoLib.h> >> >> > -#include <Base.h> >> >> > -#include <Library/PcdLib.h> >> >> > -#include <AutoGen.h> >> >> > >> >> > #include <Chipset/ArmV7.h> >> >> > >> >> > -.text >> >> > -.align 3 >> >> > - >> >> > -GCC_ASM_IMPORT(CEntryPoint) >> >> > -GCC_ASM_IMPORT(ArmPlatformIsPrimaryCore) >> >> > -GCC_ASM_IMPORT(ArmReadMpidr) >> >> > -GCC_ASM_IMPORT(ArmPlatformPeiBootAction) >> >> > -GCC_ASM_IMPORT(ArmPlatformStackSet) >> >> > -GCC_ASM_EXPORT(_ModuleEntryPoint) >> >> > GCC_ASM_EXPORT(mSystemMemoryEnd) >> >> > >> >> > -StartupAddr: .word CEntryPoint >> >> > -mSystemMemoryEnd: .8byte 0 >> >> > - >> >> > - >> >> > -ASM_PFX(_ModuleEntryPoint): >> >> > +ASM_FUNC(_ModuleEntryPoint) >> >> > // Do early platform specific actions >> >> > bl ASM_PFX(ArmPlatformPeiBootAction) >> >> > >> >> > @@ -57,10 +41,8 @@ _SystemMemoryEndInit: >> >> > cmp r1, #0 >> >> > bne _SetupStackPosition >> >> > >> >> > - LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryBase), r1) >> >> > - LoadConstantToReg (FixedPcdGet32(PcdSystemMemorySize), r2) >> >> > - sub r2, r2, #1 >> >> > - add r1, r1, r2 >> >> > + MOV32 (r1, FixedPcdGet32(PcdSystemMemoryBase) + >> >> > FixedPcdGet32(PcdSystemMemorySize) - 1) >> >> > + >> >> > // Update the global variable >> >> > adr r2, mSystemMemoryEnd >> >> > str r1, [r2] >> >> > @@ -69,13 +51,12 @@ _SetupStackPosition: >> >> > // r1 = SystemMemoryTop >> >> > >> >> > // Calculate Top of the Firmware Device >> >> > - LoadConstantToReg (FixedPcdGet32(PcdFdBaseAddress), r2) >> >> > - LoadConstantToReg (FixedPcdGet32(PcdFdSize), r3) >> >> > - sub r3, r3, #1 >> >> > + 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) >> >> > - LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryUefiRegionSize), >> >> > r4) >> >> > + MOV32 (r4, FixedPcdGet32(PcdSystemMemoryUefiRegionSize)) >> >> > >> >> > // >> >> > // Reserve the memory for the UEFI region (contain stacks on its >> >> > top) >> >> > @@ -106,9 +87,8 @@ _SetupAlignedStack: >> >> > _SetupOverflowStack: >> >> > // Case memory at the top of the address space. Ensure the top of >> >> > the >> >> > stack is EFI_PAGE_SIZE >> >> > // aligned (4KB) >> >> > - LoadConstantToReg (EFI_PAGE_MASK, r9) >> >> > - and r9, r9, r1 >> >> > - sub r1, r1, r9 >> >> > + MOV32 (r9, ~EFI_PAGE_MASK & 0xFFFFFFFF) >> >> > + and r1, r1, r9 >> >> > >> >> > _GetBaseUefiMemory: >> >> > // Calculate the Base of the UEFI Memory >> >> > @@ -117,22 +97,19 @@ _GetBaseUefiMemory: >> >> > _GetStackBase: >> >> > // r1 = The top of the Mpcore Stacks >> >> > // Stack for the primary core = PrimaryCoreStack >> >> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), r2) >> >> > + MOV32 (r2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) >> >> > sub r10, r1, r2 >> >> > >> >> > // Stack for the secondary core = Number of Cores - 1 >> >> > - LoadConstantToReg (FixedPcdGet32(PcdCoreCount), r0) >> >> > - sub r0, r0, #1 >> >> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), >> >> > r1) >> >> > - mul r1, r1, r0 >> >> > + MOV32 (r0, (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) >> >> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), r2) >> >> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), >> >> > r3) >> >> > + MOV32 (r2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) >> >> > + MOV32 (r3, FixedPcdGet32(PcdCPUCoreSecondaryStackSize)) >> >> > bl ASM_PFX(ArmPlatformStackSet) >> >> > >> >> > // Is it the Primary Core ? >> >> > @@ -149,7 +126,7 @@ _PrepareArguments: >> >> > >> >> > // 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 >> >> > + ldr r4, =ASM_PFX(CEntryPoint) >> >> > >> >> > // Jump to PrePiCore C code >> >> > // r0 = MpId >> >> > @@ -160,3 +137,4 @@ _PrepareArguments: >> >> > _NeverReturn: >> >> > b _NeverReturn >> >> > >> >> > +ASM_PFX(mSystemMemoryEnd): .8byte 0 >> >> > -- >> >> > 2.7.4 >> >> > >> >> _______________________________________________ >> >> edk2-devel mailing list >> >> edk2-devel@lists.01.org >> >> https://lists.01.org/mailman/listinfo/edk2-devel >> > >> > > > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
>-----Original Message----- >From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of >Michael Zimmermann >Sent: 31 August 2016 05:33 >To: Leif Lindholm >Cc: edk2-devel@lists.01.org; Cohen, Eugene; Laszlo Ersek; >ard.biesheuvel@linaro.org >Subject: Re: [edk2] [PATCH 22/26] ArmPlatformPkg/PrePi: switch to >ASM_FUNC() asm macro > >reviewed should mean tested ;) > Sorry to be pedantic, but shouldn't "Reviewed-by" mean code reviewed, and "Tested-by" mean tested? Evan ... IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 31 August 2016 at 10:52, Michael Zimmermann <sigmaepsilon92@gmail.com> wrote: >>Sorry to be pedantic, but shouldn't "Reviewed-by" mean code reviewed, and >> "Tested-by" > mean tested? > Yes you're right, sorry. I guess I just had too much hope in reviewers > actually testing the changes :D > You mean those reviewers that actually own a BeagleBoard? > On Wed, Aug 31, 2016 at 11:47 AM, Evan Lloyd <Evan.Lloyd@arm.com> wrote: >> >> >-----Original Message----- >> >From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of >> >Michael Zimmermann >> >Sent: 31 August 2016 05:33 >> >To: Leif Lindholm >> >Cc: edk2-devel@lists.01.org; Cohen, Eugene; Laszlo Ersek; >> >ard.biesheuvel@linaro.org >> >Subject: Re: [edk2] [PATCH 22/26] ArmPlatformPkg/PrePi: switch to >> >ASM_FUNC() asm macro >> > >> >reviewed should mean tested ;) >> > >> Sorry to be pedantic, but shouldn't "Reviewed-by" mean code reviewed, and >> "Tested-by" mean tested? >> >> Evan >> ... >> >> IMPORTANT NOTICE: The contents of this email and any attachments are >> confidential and may also be privileged. If you are not the intended >> recipient, please notify the sender immediately and do not disclose the >> contents to any other person, use it for any purpose, or store or copy the >> information in any medium. Thank you. >> > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 31 August 2016 at 05:33, Michael Zimmermann <sigmaepsilon92@gmail.com> wrote: > reviewed should mean tested ;) > > took me some time to find out why my system currently doesn't boot anymore I have just bisected my TC2 boot failure down to this patch too. > but here's the fix for this commit: > > diff --git a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S > b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S > index b7127ce..39030da 100644 > --- a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S > +++ b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S > @@ -101,7 +101,7 @@ _GetStackBase: > sub r10, r1, r2 > > // Stack for the secondary core = Number of Cores - 1 > - MOV32 (r0, (FixedPcdGet32(PcdCoreCount) - 1) * > FixedPcdGet32(PcdCPUCoreSecondaryStackSize)) > + MOV32 (r1, (FixedPcdGet32(PcdCoreCount) - 1) * > FixedPcdGet32(PcdCPUCoreSecondaryStackSize))^M > sub r10, r10, r1 > > // r10 = The base of the MpCore Stacks (primary stack & secondary stacks) And this fix works for me on TC2 also. Michael/Ard, has one of you already or does one of you intend to submit the fix for this? > > On Thu, Aug 11, 2016 at 10:38 AM, Leif Lindholm <leif.lindholm@linaro.org> > wrote: > >> On Wed, Aug 10, 2016 at 05:17:58PM +0200, Ard Biesheuvel wrote: >> > Annotate functions with ASM_FUNC() so that they are emitted into >> > separate sections. >> >> Also replacing LoadConstantToReg. Add that to commit message and: >> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> >> >> > Contributed-under: TianoCore Contribution Agreement 1.0 >> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> > --- >> > ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S | 49 >> ++++++------------- >> > ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S | 50 >> ++++++-------------- >> > 2 files changed, 29 insertions(+), 70 deletions(-) >> > >> > diff --git a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S >> b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S >> > index 9538c70a237c..d0530a874726 100644 >> > --- a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S >> > +++ b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S >> > @@ -12,24 +12,10 @@ >> > // >> > >> > #include <AsmMacroIoLibV8.h> >> > -#include <Base.h> >> > -#include <Library/PcdLib.h> >> > -#include <AutoGen.h> >> > - >> > -.text >> > -.align 3 >> > - >> > -GCC_ASM_IMPORT(ArmPlatformIsPrimaryCore) >> > -GCC_ASM_IMPORT(ArmReadMpidr) >> > -GCC_ASM_IMPORT(ArmPlatformPeiBootAction) >> > -GCC_ASM_IMPORT(ArmPlatformStackSet) >> > -GCC_ASM_EXPORT(_ModuleEntryPoint) >> > -ASM_GLOBAL ASM_PFX(mSystemMemoryEnd) >> > >> > -StartupAddr: .8byte ASM_PFX(CEntryPoint) >> > -ASM_PFX(mSystemMemoryEnd): .8byte 0 >> > +ASM_GLOBAL ASM_PFX(mSystemMemoryEnd) >> > >> > -ASM_PFX(_ModuleEntryPoint): >> > +ASM_FUNC(_ModuleEntryPoint) >> > // Do early platform specific actions >> > bl ASM_PFX(ArmPlatformPeiBootAction) >> > >> > @@ -49,10 +35,8 @@ _SystemMemoryEndInit: >> > cmp x1, #0 >> > bne _SetupStackPosition >> > >> > - LoadConstantToReg (FixedPcdGet64(PcdSystemMemoryBase), x1) >> > - LoadConstantToReg (FixedPcdGet64(PcdSystemMemorySize), x2) >> > - sub x2, x2, #1 >> > - add x1, x1, x2 >> > + MOV64 (x1, FixedPcdGet64(PcdSystemMemoryBase) + FixedPcdGet64(PcdSystemMemorySize) >> - 1) >> > + >> > // Update the global variable >> > adr x2, mSystemMemoryEnd >> > str x1, [x2] >> > @@ -61,13 +45,13 @@ _SetupStackPosition: >> > // r1 = SystemMemoryTop >> > >> > // Calculate Top of the Firmware Device >> > - LoadConstantToReg (FixedPcdGet64(PcdFdBaseAddress), x2) >> > - LoadConstantToReg (FixedPcdGet32(PcdFdSize), x3) >> > + MOV64 (x2, FixedPcdGet64(PcdFdBaseAddress)) >> > + MOV32 (x3, FixedPcdGet32(PcdFdSize) - 1) >> > sub x3, x3, #1 >> > add x3, x3, x2 // x3 = FdTop = PcdFdBaseAddress + PcdFdSize >> > >> > // UEFI Memory Size (stacks are allocated in this region) >> > - LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryUefiRegionSize), x4) >> > + MOV32 (x4, FixedPcdGet32(PcdSystemMemoryUefiRegionSize)) >> > >> > // >> > // Reserve the memory for the UEFI region (contain stacks on its top) >> > @@ -98,9 +82,7 @@ _SetupAlignedStack: >> > _SetupOverflowStack: >> > // Case memory at the top of the address space. Ensure the top of the >> stack is EFI_PAGE_SIZE >> > // aligned (4KB) >> > - LoadConstantToReg (EFI_PAGE_MASK, x11) >> > - and x11, x11, x1 >> > - sub x1, x1, x11 >> > + and x1, x1, ~EFI_PAGE_MASK >> > >> > _GetBaseUefiMemory: >> > // Calculate the Base of the UEFI Memory >> > @@ -109,22 +91,19 @@ _GetBaseUefiMemory: >> > _GetStackBase: >> > // r1 = The top of the Mpcore Stacks >> > // Stack for the primary core = PrimaryCoreStack >> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), x2) >> > + MOV32 (x2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) >> > sub x12, x1, x2 >> > >> > // Stack for the secondary core = Number of Cores - 1 >> > - LoadConstantToReg (FixedPcdGet32(PcdCoreCount), x0) >> > - sub x0, x0, #1 >> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), x1) >> > - mul x1, x1, x0 >> > + MOV32 (x1, (FixedPcdGet32(PcdCoreCount) - 1) * FixedPcdGet32( >> PcdCPUCoreSecondaryStackSize)) >> > sub x12, x12, x1 >> > >> > // x12 = The base of the MpCore Stacks (primary stack & secondary >> stacks) >> > mov x0, x12 >> > mov x1, x10 >> > //ArmPlatformStackSet(StackBase, MpId, PrimaryStackSize, >> SecondaryStackSize) >> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), x2) >> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), x3) >> > + MOV32 (x2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) >> > + MOV32 (x3, FixedPcdGet32(PcdCPUCoreSecondaryStackSize)) >> > bl ASM_PFX(ArmPlatformStackSet) >> > >> > // Is it the Primary Core ? >> > @@ -140,7 +119,7 @@ _PrepareArguments: >> > >> > // Move sec startup address into a data register >> > // Ensure we're jumping to FV version of the code (not boot remapped >> alias) >> > - ldr x4, StartupAddr >> > + ldr x4, =ASM_PFX(CEntryPoint) >> > >> > // Jump to PrePiCore C code >> > // x0 = MpId >> > @@ -150,3 +129,5 @@ _PrepareArguments: >> > >> > _NeverReturn: >> > b _NeverReturn >> > + >> > +ASM_PFX(mSystemMemoryEnd): .8byte 0 >> > diff --git a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >> b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >> > index 1311efc5cb2c..b7127ce9fb4c 100644 >> > --- a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >> > +++ b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >> > @@ -12,28 +12,12 @@ >> > // >> > >> > #include <AsmMacroIoLib.h> >> > -#include <Base.h> >> > -#include <Library/PcdLib.h> >> > -#include <AutoGen.h> >> > >> > #include <Chipset/ArmV7.h> >> > >> > -.text >> > -.align 3 >> > - >> > -GCC_ASM_IMPORT(CEntryPoint) >> > -GCC_ASM_IMPORT(ArmPlatformIsPrimaryCore) >> > -GCC_ASM_IMPORT(ArmReadMpidr) >> > -GCC_ASM_IMPORT(ArmPlatformPeiBootAction) >> > -GCC_ASM_IMPORT(ArmPlatformStackSet) >> > -GCC_ASM_EXPORT(_ModuleEntryPoint) >> > GCC_ASM_EXPORT(mSystemMemoryEnd) >> > >> > -StartupAddr: .word CEntryPoint >> > -mSystemMemoryEnd: .8byte 0 >> > - >> > - >> > -ASM_PFX(_ModuleEntryPoint): >> > +ASM_FUNC(_ModuleEntryPoint) >> > // Do early platform specific actions >> > bl ASM_PFX(ArmPlatformPeiBootAction) >> > >> > @@ -57,10 +41,8 @@ _SystemMemoryEndInit: >> > cmp r1, #0 >> > bne _SetupStackPosition >> > >> > - LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryBase), r1) >> > - LoadConstantToReg (FixedPcdGet32(PcdSystemMemorySize), r2) >> > - sub r2, r2, #1 >> > - add r1, r1, r2 >> > + MOV32 (r1, FixedPcdGet32(PcdSystemMemoryBase) + FixedPcdGet32(PcdSystemMemorySize) >> - 1) >> > + >> > // Update the global variable >> > adr r2, mSystemMemoryEnd >> > str r1, [r2] >> > @@ -69,13 +51,12 @@ _SetupStackPosition: >> > // r1 = SystemMemoryTop >> > >> > // Calculate Top of the Firmware Device >> > - LoadConstantToReg (FixedPcdGet32(PcdFdBaseAddress), r2) >> > - LoadConstantToReg (FixedPcdGet32(PcdFdSize), r3) >> > - sub r3, r3, #1 >> > + 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) >> > - LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryUefiRegionSize), r4) >> > + MOV32 (r4, FixedPcdGet32(PcdSystemMemoryUefiRegionSize)) >> > >> > // >> > // Reserve the memory for the UEFI region (contain stacks on its top) >> > @@ -106,9 +87,8 @@ _SetupAlignedStack: >> > _SetupOverflowStack: >> > // Case memory at the top of the address space. Ensure the top of the >> stack is EFI_PAGE_SIZE >> > // aligned (4KB) >> > - LoadConstantToReg (EFI_PAGE_MASK, r9) >> > - and r9, r9, r1 >> > - sub r1, r1, r9 >> > + MOV32 (r9, ~EFI_PAGE_MASK & 0xFFFFFFFF) >> > + and r1, r1, r9 >> > >> > _GetBaseUefiMemory: >> > // Calculate the Base of the UEFI Memory >> > @@ -117,22 +97,19 @@ _GetBaseUefiMemory: >> > _GetStackBase: >> > // r1 = The top of the Mpcore Stacks >> > // Stack for the primary core = PrimaryCoreStack >> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), r2) >> > + MOV32 (r2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) >> > sub r10, r1, r2 >> > >> > // Stack for the secondary core = Number of Cores - 1 >> > - LoadConstantToReg (FixedPcdGet32(PcdCoreCount), r0) >> > - sub r0, r0, #1 >> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), r1) >> > - mul r1, r1, r0 >> > + MOV32 (r0, (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) >> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), r2) >> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), r3) >> > + MOV32 (r2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) >> > + MOV32 (r3, FixedPcdGet32(PcdCPUCoreSecondaryStackSize)) >> > bl ASM_PFX(ArmPlatformStackSet) >> > >> > // Is it the Primary Core ? >> > @@ -149,7 +126,7 @@ _PrepareArguments: >> > >> > // 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 >> > + ldr r4, =ASM_PFX(CEntryPoint) >> > >> > // Jump to PrePiCore C code >> > // r0 = MpId >> > @@ -160,3 +137,4 @@ _PrepareArguments: >> > _NeverReturn: >> > b _NeverReturn >> > >> > +ASM_PFX(mSystemMemoryEnd): .8byte 0 >> > -- >> > 2.7.4 >> > >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel >> > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 7 September 2016 at 12:10, Ryan Harkin <ryan.harkin@linaro.org> wrote: > On 31 August 2016 at 05:33, Michael Zimmermann <sigmaepsilon92@gmail.com> wrote: >> reviewed should mean tested ;) >> >> took me some time to find out why my system currently doesn't boot anymore > > I have just bisected my TC2 boot failure down to this patch too. > >> but here's the fix for this commit: >> >> diff --git a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >> b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >> index b7127ce..39030da 100644 >> --- a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >> +++ b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >> @@ -101,7 +101,7 @@ _GetStackBase: >> sub r10, r1, r2 >> >> // Stack for the secondary core = Number of Cores - 1 >> - MOV32 (r0, (FixedPcdGet32(PcdCoreCount) - 1) * >> FixedPcdGet32(PcdCPUCoreSecondaryStackSize)) >> + MOV32 (r1, (FixedPcdGet32(PcdCoreCount) - 1) * >> FixedPcdGet32(PcdCPUCoreSecondaryStackSize))^M >> sub r10, r10, r1 >> >> // r10 = The base of the MpCore Stacks (primary stack & secondary stacks) > > And this fix works for me on TC2 also. > May we take that as a tested-by? > Michael/Ard, has one of you already or does one of you intend to > submit the fix for this? > Michael: care to send it as a proper patch, please? > >> >> On Thu, Aug 11, 2016 at 10:38 AM, Leif Lindholm <leif.lindholm@linaro.org> >> wrote: >> >>> On Wed, Aug 10, 2016 at 05:17:58PM +0200, Ard Biesheuvel wrote: >>> > Annotate functions with ASM_FUNC() so that they are emitted into >>> > separate sections. >>> >>> Also replacing LoadConstantToReg. Add that to commit message and: >>> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> >>> >>> > Contributed-under: TianoCore Contribution Agreement 1.0 >>> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >>> > --- >>> > ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S | 49 >>> ++++++------------- >>> > ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S | 50 >>> ++++++-------------- >>> > 2 files changed, 29 insertions(+), 70 deletions(-) >>> > >>> > diff --git a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S >>> b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S >>> > index 9538c70a237c..d0530a874726 100644 >>> > --- a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S >>> > +++ b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S >>> > @@ -12,24 +12,10 @@ >>> > // >>> > >>> > #include <AsmMacroIoLibV8.h> >>> > -#include <Base.h> >>> > -#include <Library/PcdLib.h> >>> > -#include <AutoGen.h> >>> > - >>> > -.text >>> > -.align 3 >>> > - >>> > -GCC_ASM_IMPORT(ArmPlatformIsPrimaryCore) >>> > -GCC_ASM_IMPORT(ArmReadMpidr) >>> > -GCC_ASM_IMPORT(ArmPlatformPeiBootAction) >>> > -GCC_ASM_IMPORT(ArmPlatformStackSet) >>> > -GCC_ASM_EXPORT(_ModuleEntryPoint) >>> > -ASM_GLOBAL ASM_PFX(mSystemMemoryEnd) >>> > >>> > -StartupAddr: .8byte ASM_PFX(CEntryPoint) >>> > -ASM_PFX(mSystemMemoryEnd): .8byte 0 >>> > +ASM_GLOBAL ASM_PFX(mSystemMemoryEnd) >>> > >>> > -ASM_PFX(_ModuleEntryPoint): >>> > +ASM_FUNC(_ModuleEntryPoint) >>> > // Do early platform specific actions >>> > bl ASM_PFX(ArmPlatformPeiBootAction) >>> > >>> > @@ -49,10 +35,8 @@ _SystemMemoryEndInit: >>> > cmp x1, #0 >>> > bne _SetupStackPosition >>> > >>> > - LoadConstantToReg (FixedPcdGet64(PcdSystemMemoryBase), x1) >>> > - LoadConstantToReg (FixedPcdGet64(PcdSystemMemorySize), x2) >>> > - sub x2, x2, #1 >>> > - add x1, x1, x2 >>> > + MOV64 (x1, FixedPcdGet64(PcdSystemMemoryBase) + FixedPcdGet64(PcdSystemMemorySize) >>> - 1) >>> > + >>> > // Update the global variable >>> > adr x2, mSystemMemoryEnd >>> > str x1, [x2] >>> > @@ -61,13 +45,13 @@ _SetupStackPosition: >>> > // r1 = SystemMemoryTop >>> > >>> > // Calculate Top of the Firmware Device >>> > - LoadConstantToReg (FixedPcdGet64(PcdFdBaseAddress), x2) >>> > - LoadConstantToReg (FixedPcdGet32(PcdFdSize), x3) >>> > + MOV64 (x2, FixedPcdGet64(PcdFdBaseAddress)) >>> > + MOV32 (x3, FixedPcdGet32(PcdFdSize) - 1) >>> > sub x3, x3, #1 >>> > add x3, x3, x2 // x3 = FdTop = PcdFdBaseAddress + PcdFdSize >>> > >>> > // UEFI Memory Size (stacks are allocated in this region) >>> > - LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryUefiRegionSize), x4) >>> > + MOV32 (x4, FixedPcdGet32(PcdSystemMemoryUefiRegionSize)) >>> > >>> > // >>> > // Reserve the memory for the UEFI region (contain stacks on its top) >>> > @@ -98,9 +82,7 @@ _SetupAlignedStack: >>> > _SetupOverflowStack: >>> > // Case memory at the top of the address space. Ensure the top of the >>> stack is EFI_PAGE_SIZE >>> > // aligned (4KB) >>> > - LoadConstantToReg (EFI_PAGE_MASK, x11) >>> > - and x11, x11, x1 >>> > - sub x1, x1, x11 >>> > + and x1, x1, ~EFI_PAGE_MASK >>> > >>> > _GetBaseUefiMemory: >>> > // Calculate the Base of the UEFI Memory >>> > @@ -109,22 +91,19 @@ _GetBaseUefiMemory: >>> > _GetStackBase: >>> > // r1 = The top of the Mpcore Stacks >>> > // Stack for the primary core = PrimaryCoreStack >>> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), x2) >>> > + MOV32 (x2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) >>> > sub x12, x1, x2 >>> > >>> > // Stack for the secondary core = Number of Cores - 1 >>> > - LoadConstantToReg (FixedPcdGet32(PcdCoreCount), x0) >>> > - sub x0, x0, #1 >>> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), x1) >>> > - mul x1, x1, x0 >>> > + MOV32 (x1, (FixedPcdGet32(PcdCoreCount) - 1) * FixedPcdGet32( >>> PcdCPUCoreSecondaryStackSize)) >>> > sub x12, x12, x1 >>> > >>> > // x12 = The base of the MpCore Stacks (primary stack & secondary >>> stacks) >>> > mov x0, x12 >>> > mov x1, x10 >>> > //ArmPlatformStackSet(StackBase, MpId, PrimaryStackSize, >>> SecondaryStackSize) >>> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), x2) >>> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), x3) >>> > + MOV32 (x2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) >>> > + MOV32 (x3, FixedPcdGet32(PcdCPUCoreSecondaryStackSize)) >>> > bl ASM_PFX(ArmPlatformStackSet) >>> > >>> > // Is it the Primary Core ? >>> > @@ -140,7 +119,7 @@ _PrepareArguments: >>> > >>> > // Move sec startup address into a data register >>> > // Ensure we're jumping to FV version of the code (not boot remapped >>> alias) >>> > - ldr x4, StartupAddr >>> > + ldr x4, =ASM_PFX(CEntryPoint) >>> > >>> > // Jump to PrePiCore C code >>> > // x0 = MpId >>> > @@ -150,3 +129,5 @@ _PrepareArguments: >>> > >>> > _NeverReturn: >>> > b _NeverReturn >>> > + >>> > +ASM_PFX(mSystemMemoryEnd): .8byte 0 >>> > diff --git a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >>> b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >>> > index 1311efc5cb2c..b7127ce9fb4c 100644 >>> > --- a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >>> > +++ b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >>> > @@ -12,28 +12,12 @@ >>> > // >>> > >>> > #include <AsmMacroIoLib.h> >>> > -#include <Base.h> >>> > -#include <Library/PcdLib.h> >>> > -#include <AutoGen.h> >>> > >>> > #include <Chipset/ArmV7.h> >>> > >>> > -.text >>> > -.align 3 >>> > - >>> > -GCC_ASM_IMPORT(CEntryPoint) >>> > -GCC_ASM_IMPORT(ArmPlatformIsPrimaryCore) >>> > -GCC_ASM_IMPORT(ArmReadMpidr) >>> > -GCC_ASM_IMPORT(ArmPlatformPeiBootAction) >>> > -GCC_ASM_IMPORT(ArmPlatformStackSet) >>> > -GCC_ASM_EXPORT(_ModuleEntryPoint) >>> > GCC_ASM_EXPORT(mSystemMemoryEnd) >>> > >>> > -StartupAddr: .word CEntryPoint >>> > -mSystemMemoryEnd: .8byte 0 >>> > - >>> > - >>> > -ASM_PFX(_ModuleEntryPoint): >>> > +ASM_FUNC(_ModuleEntryPoint) >>> > // Do early platform specific actions >>> > bl ASM_PFX(ArmPlatformPeiBootAction) >>> > >>> > @@ -57,10 +41,8 @@ _SystemMemoryEndInit: >>> > cmp r1, #0 >>> > bne _SetupStackPosition >>> > >>> > - LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryBase), r1) >>> > - LoadConstantToReg (FixedPcdGet32(PcdSystemMemorySize), r2) >>> > - sub r2, r2, #1 >>> > - add r1, r1, r2 >>> > + MOV32 (r1, FixedPcdGet32(PcdSystemMemoryBase) + FixedPcdGet32(PcdSystemMemorySize) >>> - 1) >>> > + >>> > // Update the global variable >>> > adr r2, mSystemMemoryEnd >>> > str r1, [r2] >>> > @@ -69,13 +51,12 @@ _SetupStackPosition: >>> > // r1 = SystemMemoryTop >>> > >>> > // Calculate Top of the Firmware Device >>> > - LoadConstantToReg (FixedPcdGet32(PcdFdBaseAddress), r2) >>> > - LoadConstantToReg (FixedPcdGet32(PcdFdSize), r3) >>> > - sub r3, r3, #1 >>> > + 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) >>> > - LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryUefiRegionSize), r4) >>> > + MOV32 (r4, FixedPcdGet32(PcdSystemMemoryUefiRegionSize)) >>> > >>> > // >>> > // Reserve the memory for the UEFI region (contain stacks on its top) >>> > @@ -106,9 +87,8 @@ _SetupAlignedStack: >>> > _SetupOverflowStack: >>> > // Case memory at the top of the address space. Ensure the top of the >>> stack is EFI_PAGE_SIZE >>> > // aligned (4KB) >>> > - LoadConstantToReg (EFI_PAGE_MASK, r9) >>> > - and r9, r9, r1 >>> > - sub r1, r1, r9 >>> > + MOV32 (r9, ~EFI_PAGE_MASK & 0xFFFFFFFF) >>> > + and r1, r1, r9 >>> > >>> > _GetBaseUefiMemory: >>> > // Calculate the Base of the UEFI Memory >>> > @@ -117,22 +97,19 @@ _GetBaseUefiMemory: >>> > _GetStackBase: >>> > // r1 = The top of the Mpcore Stacks >>> > // Stack for the primary core = PrimaryCoreStack >>> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), r2) >>> > + MOV32 (r2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) >>> > sub r10, r1, r2 >>> > >>> > // Stack for the secondary core = Number of Cores - 1 >>> > - LoadConstantToReg (FixedPcdGet32(PcdCoreCount), r0) >>> > - sub r0, r0, #1 >>> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), r1) >>> > - mul r1, r1, r0 >>> > + MOV32 (r0, (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) >>> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), r2) >>> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), r3) >>> > + MOV32 (r2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) >>> > + MOV32 (r3, FixedPcdGet32(PcdCPUCoreSecondaryStackSize)) >>> > bl ASM_PFX(ArmPlatformStackSet) >>> > >>> > // Is it the Primary Core ? >>> > @@ -149,7 +126,7 @@ _PrepareArguments: >>> > >>> > // 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 >>> > + ldr r4, =ASM_PFX(CEntryPoint) >>> > >>> > // Jump to PrePiCore C code >>> > // r0 = MpId >>> > @@ -160,3 +137,4 @@ _PrepareArguments: >>> > _NeverReturn: >>> > b _NeverReturn >>> > >>> > +ASM_PFX(mSystemMemoryEnd): .8byte 0 >>> > -- >>> > 2.7.4 >>> > >>> _______________________________________________ >>> edk2-devel mailing list >>> edk2-devel@lists.01.org >>> https://lists.01.org/mailman/listinfo/edk2-devel >>> >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 7 September 2016 at 12:59, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 7 September 2016 at 12:10, Ryan Harkin <ryan.harkin@linaro.org> wrote: >> On 31 August 2016 at 05:33, Michael Zimmermann <sigmaepsilon92@gmail.com> wrote: >>> reviewed should mean tested ;) >>> >>> took me some time to find out why my system currently doesn't boot anymore >> >> I have just bisected my TC2 boot failure down to this patch too. >> >>> but here's the fix for this commit: >>> >>> diff --git a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >>> b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >>> index b7127ce..39030da 100644 >>> --- a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >>> +++ b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >>> @@ -101,7 +101,7 @@ _GetStackBase: >>> sub r10, r1, r2 >>> >>> // Stack for the secondary core = Number of Cores - 1 >>> - MOV32 (r0, (FixedPcdGet32(PcdCoreCount) - 1) * >>> FixedPcdGet32(PcdCPUCoreSecondaryStackSize)) >>> + MOV32 (r1, (FixedPcdGet32(PcdCoreCount) - 1) * >>> FixedPcdGet32(PcdCPUCoreSecondaryStackSize))^M >>> sub r10, r10, r1 >>> >>> // r10 = The base of the MpCore Stacks (primary stack & secondary stacks) >> >> And this fix works for me on TC2 also. >> > > May we take that as a tested-by? > Oh yes! :-) You can even have some tested text too: Tested-by: Ryan Harkin <ryan.harkin@linaro.org> But I'll be happy to report that if Michael (or someone else) posts a patch. >> Michael/Ard, has one of you already or does one of you intend to >> submit the fix for this? >> > > Michael: care to send it as a proper patch, please? > > >> >>> >>> On Thu, Aug 11, 2016 at 10:38 AM, Leif Lindholm <leif.lindholm@linaro.org> >>> wrote: >>> >>>> On Wed, Aug 10, 2016 at 05:17:58PM +0200, Ard Biesheuvel wrote: >>>> > Annotate functions with ASM_FUNC() so that they are emitted into >>>> > separate sections. >>>> >>>> Also replacing LoadConstantToReg. Add that to commit message and: >>>> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> >>>> >>>> > Contributed-under: TianoCore Contribution Agreement 1.0 >>>> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >>>> > --- >>>> > ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S | 49 >>>> ++++++------------- >>>> > ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S | 50 >>>> ++++++-------------- >>>> > 2 files changed, 29 insertions(+), 70 deletions(-) >>>> > >>>> > diff --git a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S >>>> b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S >>>> > index 9538c70a237c..d0530a874726 100644 >>>> > --- a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S >>>> > +++ b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S >>>> > @@ -12,24 +12,10 @@ >>>> > // >>>> > >>>> > #include <AsmMacroIoLibV8.h> >>>> > -#include <Base.h> >>>> > -#include <Library/PcdLib.h> >>>> > -#include <AutoGen.h> >>>> > - >>>> > -.text >>>> > -.align 3 >>>> > - >>>> > -GCC_ASM_IMPORT(ArmPlatformIsPrimaryCore) >>>> > -GCC_ASM_IMPORT(ArmReadMpidr) >>>> > -GCC_ASM_IMPORT(ArmPlatformPeiBootAction) >>>> > -GCC_ASM_IMPORT(ArmPlatformStackSet) >>>> > -GCC_ASM_EXPORT(_ModuleEntryPoint) >>>> > -ASM_GLOBAL ASM_PFX(mSystemMemoryEnd) >>>> > >>>> > -StartupAddr: .8byte ASM_PFX(CEntryPoint) >>>> > -ASM_PFX(mSystemMemoryEnd): .8byte 0 >>>> > +ASM_GLOBAL ASM_PFX(mSystemMemoryEnd) >>>> > >>>> > -ASM_PFX(_ModuleEntryPoint): >>>> > +ASM_FUNC(_ModuleEntryPoint) >>>> > // Do early platform specific actions >>>> > bl ASM_PFX(ArmPlatformPeiBootAction) >>>> > >>>> > @@ -49,10 +35,8 @@ _SystemMemoryEndInit: >>>> > cmp x1, #0 >>>> > bne _SetupStackPosition >>>> > >>>> > - LoadConstantToReg (FixedPcdGet64(PcdSystemMemoryBase), x1) >>>> > - LoadConstantToReg (FixedPcdGet64(PcdSystemMemorySize), x2) >>>> > - sub x2, x2, #1 >>>> > - add x1, x1, x2 >>>> > + MOV64 (x1, FixedPcdGet64(PcdSystemMemoryBase) + FixedPcdGet64(PcdSystemMemorySize) >>>> - 1) >>>> > + >>>> > // Update the global variable >>>> > adr x2, mSystemMemoryEnd >>>> > str x1, [x2] >>>> > @@ -61,13 +45,13 @@ _SetupStackPosition: >>>> > // r1 = SystemMemoryTop >>>> > >>>> > // Calculate Top of the Firmware Device >>>> > - LoadConstantToReg (FixedPcdGet64(PcdFdBaseAddress), x2) >>>> > - LoadConstantToReg (FixedPcdGet32(PcdFdSize), x3) >>>> > + MOV64 (x2, FixedPcdGet64(PcdFdBaseAddress)) >>>> > + MOV32 (x3, FixedPcdGet32(PcdFdSize) - 1) >>>> > sub x3, x3, #1 >>>> > add x3, x3, x2 // x3 = FdTop = PcdFdBaseAddress + PcdFdSize >>>> > >>>> > // UEFI Memory Size (stacks are allocated in this region) >>>> > - LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryUefiRegionSize), x4) >>>> > + MOV32 (x4, FixedPcdGet32(PcdSystemMemoryUefiRegionSize)) >>>> > >>>> > // >>>> > // Reserve the memory for the UEFI region (contain stacks on its top) >>>> > @@ -98,9 +82,7 @@ _SetupAlignedStack: >>>> > _SetupOverflowStack: >>>> > // Case memory at the top of the address space. Ensure the top of the >>>> stack is EFI_PAGE_SIZE >>>> > // aligned (4KB) >>>> > - LoadConstantToReg (EFI_PAGE_MASK, x11) >>>> > - and x11, x11, x1 >>>> > - sub x1, x1, x11 >>>> > + and x1, x1, ~EFI_PAGE_MASK >>>> > >>>> > _GetBaseUefiMemory: >>>> > // Calculate the Base of the UEFI Memory >>>> > @@ -109,22 +91,19 @@ _GetBaseUefiMemory: >>>> > _GetStackBase: >>>> > // r1 = The top of the Mpcore Stacks >>>> > // Stack for the primary core = PrimaryCoreStack >>>> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), x2) >>>> > + MOV32 (x2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) >>>> > sub x12, x1, x2 >>>> > >>>> > // Stack for the secondary core = Number of Cores - 1 >>>> > - LoadConstantToReg (FixedPcdGet32(PcdCoreCount), x0) >>>> > - sub x0, x0, #1 >>>> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), x1) >>>> > - mul x1, x1, x0 >>>> > + MOV32 (x1, (FixedPcdGet32(PcdCoreCount) - 1) * FixedPcdGet32( >>>> PcdCPUCoreSecondaryStackSize)) >>>> > sub x12, x12, x1 >>>> > >>>> > // x12 = The base of the MpCore Stacks (primary stack & secondary >>>> stacks) >>>> > mov x0, x12 >>>> > mov x1, x10 >>>> > //ArmPlatformStackSet(StackBase, MpId, PrimaryStackSize, >>>> SecondaryStackSize) >>>> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), x2) >>>> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), x3) >>>> > + MOV32 (x2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) >>>> > + MOV32 (x3, FixedPcdGet32(PcdCPUCoreSecondaryStackSize)) >>>> > bl ASM_PFX(ArmPlatformStackSet) >>>> > >>>> > // Is it the Primary Core ? >>>> > @@ -140,7 +119,7 @@ _PrepareArguments: >>>> > >>>> > // Move sec startup address into a data register >>>> > // Ensure we're jumping to FV version of the code (not boot remapped >>>> alias) >>>> > - ldr x4, StartupAddr >>>> > + ldr x4, =ASM_PFX(CEntryPoint) >>>> > >>>> > // Jump to PrePiCore C code >>>> > // x0 = MpId >>>> > @@ -150,3 +129,5 @@ _PrepareArguments: >>>> > >>>> > _NeverReturn: >>>> > b _NeverReturn >>>> > + >>>> > +ASM_PFX(mSystemMemoryEnd): .8byte 0 >>>> > diff --git a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >>>> b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >>>> > index 1311efc5cb2c..b7127ce9fb4c 100644 >>>> > --- a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >>>> > +++ b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >>>> > @@ -12,28 +12,12 @@ >>>> > // >>>> > >>>> > #include <AsmMacroIoLib.h> >>>> > -#include <Base.h> >>>> > -#include <Library/PcdLib.h> >>>> > -#include <AutoGen.h> >>>> > >>>> > #include <Chipset/ArmV7.h> >>>> > >>>> > -.text >>>> > -.align 3 >>>> > - >>>> > -GCC_ASM_IMPORT(CEntryPoint) >>>> > -GCC_ASM_IMPORT(ArmPlatformIsPrimaryCore) >>>> > -GCC_ASM_IMPORT(ArmReadMpidr) >>>> > -GCC_ASM_IMPORT(ArmPlatformPeiBootAction) >>>> > -GCC_ASM_IMPORT(ArmPlatformStackSet) >>>> > -GCC_ASM_EXPORT(_ModuleEntryPoint) >>>> > GCC_ASM_EXPORT(mSystemMemoryEnd) >>>> > >>>> > -StartupAddr: .word CEntryPoint >>>> > -mSystemMemoryEnd: .8byte 0 >>>> > - >>>> > - >>>> > -ASM_PFX(_ModuleEntryPoint): >>>> > +ASM_FUNC(_ModuleEntryPoint) >>>> > // Do early platform specific actions >>>> > bl ASM_PFX(ArmPlatformPeiBootAction) >>>> > >>>> > @@ -57,10 +41,8 @@ _SystemMemoryEndInit: >>>> > cmp r1, #0 >>>> > bne _SetupStackPosition >>>> > >>>> > - LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryBase), r1) >>>> > - LoadConstantToReg (FixedPcdGet32(PcdSystemMemorySize), r2) >>>> > - sub r2, r2, #1 >>>> > - add r1, r1, r2 >>>> > + MOV32 (r1, FixedPcdGet32(PcdSystemMemoryBase) + FixedPcdGet32(PcdSystemMemorySize) >>>> - 1) >>>> > + >>>> > // Update the global variable >>>> > adr r2, mSystemMemoryEnd >>>> > str r1, [r2] >>>> > @@ -69,13 +51,12 @@ _SetupStackPosition: >>>> > // r1 = SystemMemoryTop >>>> > >>>> > // Calculate Top of the Firmware Device >>>> > - LoadConstantToReg (FixedPcdGet32(PcdFdBaseAddress), r2) >>>> > - LoadConstantToReg (FixedPcdGet32(PcdFdSize), r3) >>>> > - sub r3, r3, #1 >>>> > + 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) >>>> > - LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryUefiRegionSize), r4) >>>> > + MOV32 (r4, FixedPcdGet32(PcdSystemMemoryUefiRegionSize)) >>>> > >>>> > // >>>> > // Reserve the memory for the UEFI region (contain stacks on its top) >>>> > @@ -106,9 +87,8 @@ _SetupAlignedStack: >>>> > _SetupOverflowStack: >>>> > // Case memory at the top of the address space. Ensure the top of the >>>> stack is EFI_PAGE_SIZE >>>> > // aligned (4KB) >>>> > - LoadConstantToReg (EFI_PAGE_MASK, r9) >>>> > - and r9, r9, r1 >>>> > - sub r1, r1, r9 >>>> > + MOV32 (r9, ~EFI_PAGE_MASK & 0xFFFFFFFF) >>>> > + and r1, r1, r9 >>>> > >>>> > _GetBaseUefiMemory: >>>> > // Calculate the Base of the UEFI Memory >>>> > @@ -117,22 +97,19 @@ _GetBaseUefiMemory: >>>> > _GetStackBase: >>>> > // r1 = The top of the Mpcore Stacks >>>> > // Stack for the primary core = PrimaryCoreStack >>>> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), r2) >>>> > + MOV32 (r2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) >>>> > sub r10, r1, r2 >>>> > >>>> > // Stack for the secondary core = Number of Cores - 1 >>>> > - LoadConstantToReg (FixedPcdGet32(PcdCoreCount), r0) >>>> > - sub r0, r0, #1 >>>> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), r1) >>>> > - mul r1, r1, r0 >>>> > + MOV32 (r0, (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) >>>> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), r2) >>>> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), r3) >>>> > + MOV32 (r2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) >>>> > + MOV32 (r3, FixedPcdGet32(PcdCPUCoreSecondaryStackSize)) >>>> > bl ASM_PFX(ArmPlatformStackSet) >>>> > >>>> > // Is it the Primary Core ? >>>> > @@ -149,7 +126,7 @@ _PrepareArguments: >>>> > >>>> > // 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 >>>> > + ldr r4, =ASM_PFX(CEntryPoint) >>>> > >>>> > // Jump to PrePiCore C code >>>> > // r0 = MpId >>>> > @@ -160,3 +137,4 @@ _PrepareArguments: >>>> > _NeverReturn: >>>> > b _NeverReturn >>>> > >>>> > +ASM_PFX(mSystemMemoryEnd): .8byte 0 >>>> > -- >>>> > 2.7.4 >>>> > >>>> _______________________________________________ >>>> edk2-devel mailing list >>>> edk2-devel@lists.01.org >>>> https://lists.01.org/mailman/listinfo/edk2-devel >>>> >>> _______________________________________________ >>> edk2-devel mailing list >>> edk2-devel@lists.01.org >>> https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S index 9538c70a237c..d0530a874726 100644 --- a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S +++ b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S @@ -12,24 +12,10 @@ // #include <AsmMacroIoLibV8.h> -#include <Base.h> -#include <Library/PcdLib.h> -#include <AutoGen.h> - -.text -.align 3 - -GCC_ASM_IMPORT(ArmPlatformIsPrimaryCore) -GCC_ASM_IMPORT(ArmReadMpidr) -GCC_ASM_IMPORT(ArmPlatformPeiBootAction) -GCC_ASM_IMPORT(ArmPlatformStackSet) -GCC_ASM_EXPORT(_ModuleEntryPoint) -ASM_GLOBAL ASM_PFX(mSystemMemoryEnd) -StartupAddr: .8byte ASM_PFX(CEntryPoint) -ASM_PFX(mSystemMemoryEnd): .8byte 0 +ASM_GLOBAL ASM_PFX(mSystemMemoryEnd) -ASM_PFX(_ModuleEntryPoint): +ASM_FUNC(_ModuleEntryPoint) // Do early platform specific actions bl ASM_PFX(ArmPlatformPeiBootAction) @@ -49,10 +35,8 @@ _SystemMemoryEndInit: cmp x1, #0 bne _SetupStackPosition - LoadConstantToReg (FixedPcdGet64(PcdSystemMemoryBase), x1) - LoadConstantToReg (FixedPcdGet64(PcdSystemMemorySize), x2) - sub x2, x2, #1 - add x1, x1, x2 + MOV64 (x1, FixedPcdGet64(PcdSystemMemoryBase) + FixedPcdGet64(PcdSystemMemorySize) - 1) + // Update the global variable adr x2, mSystemMemoryEnd str x1, [x2] @@ -61,13 +45,13 @@ _SetupStackPosition: // r1 = SystemMemoryTop // Calculate Top of the Firmware Device - LoadConstantToReg (FixedPcdGet64(PcdFdBaseAddress), x2) - LoadConstantToReg (FixedPcdGet32(PcdFdSize), x3) + MOV64 (x2, FixedPcdGet64(PcdFdBaseAddress)) + MOV32 (x3, FixedPcdGet32(PcdFdSize) - 1) sub x3, x3, #1 add x3, x3, x2 // x3 = FdTop = PcdFdBaseAddress + PcdFdSize // UEFI Memory Size (stacks are allocated in this region) - LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryUefiRegionSize), x4) + MOV32 (x4, FixedPcdGet32(PcdSystemMemoryUefiRegionSize)) // // Reserve the memory for the UEFI region (contain stacks on its top) @@ -98,9 +82,7 @@ _SetupAlignedStack: _SetupOverflowStack: // Case memory at the top of the address space. Ensure the top of the stack is EFI_PAGE_SIZE // aligned (4KB) - LoadConstantToReg (EFI_PAGE_MASK, x11) - and x11, x11, x1 - sub x1, x1, x11 + and x1, x1, ~EFI_PAGE_MASK _GetBaseUefiMemory: // Calculate the Base of the UEFI Memory @@ -109,22 +91,19 @@ _GetBaseUefiMemory: _GetStackBase: // r1 = The top of the Mpcore Stacks // Stack for the primary core = PrimaryCoreStack - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), x2) + MOV32 (x2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) sub x12, x1, x2 // Stack for the secondary core = Number of Cores - 1 - LoadConstantToReg (FixedPcdGet32(PcdCoreCount), x0) - sub x0, x0, #1 - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), x1) - mul x1, x1, x0 + MOV32 (x1, (FixedPcdGet32(PcdCoreCount) - 1) * FixedPcdGet32(PcdCPUCoreSecondaryStackSize)) sub x12, x12, x1 // x12 = The base of the MpCore Stacks (primary stack & secondary stacks) mov x0, x12 mov x1, x10 //ArmPlatformStackSet(StackBase, MpId, PrimaryStackSize, SecondaryStackSize) - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), x2) - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), x3) + MOV32 (x2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) + MOV32 (x3, FixedPcdGet32(PcdCPUCoreSecondaryStackSize)) bl ASM_PFX(ArmPlatformStackSet) // Is it the Primary Core ? @@ -140,7 +119,7 @@ _PrepareArguments: // Move sec startup address into a data register // Ensure we're jumping to FV version of the code (not boot remapped alias) - ldr x4, StartupAddr + ldr x4, =ASM_PFX(CEntryPoint) // Jump to PrePiCore C code // x0 = MpId @@ -150,3 +129,5 @@ _PrepareArguments: _NeverReturn: b _NeverReturn + +ASM_PFX(mSystemMemoryEnd): .8byte 0 diff --git a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S index 1311efc5cb2c..b7127ce9fb4c 100644 --- a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S +++ b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S @@ -12,28 +12,12 @@ // #include <AsmMacroIoLib.h> -#include <Base.h> -#include <Library/PcdLib.h> -#include <AutoGen.h> #include <Chipset/ArmV7.h> -.text -.align 3 - -GCC_ASM_IMPORT(CEntryPoint) -GCC_ASM_IMPORT(ArmPlatformIsPrimaryCore) -GCC_ASM_IMPORT(ArmReadMpidr) -GCC_ASM_IMPORT(ArmPlatformPeiBootAction) -GCC_ASM_IMPORT(ArmPlatformStackSet) -GCC_ASM_EXPORT(_ModuleEntryPoint) GCC_ASM_EXPORT(mSystemMemoryEnd) -StartupAddr: .word CEntryPoint -mSystemMemoryEnd: .8byte 0 - - -ASM_PFX(_ModuleEntryPoint): +ASM_FUNC(_ModuleEntryPoint) // Do early platform specific actions bl ASM_PFX(ArmPlatformPeiBootAction) @@ -57,10 +41,8 @@ _SystemMemoryEndInit: cmp r1, #0 bne _SetupStackPosition - LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryBase), r1) - LoadConstantToReg (FixedPcdGet32(PcdSystemMemorySize), r2) - sub r2, r2, #1 - add r1, r1, r2 + MOV32 (r1, FixedPcdGet32(PcdSystemMemoryBase) + FixedPcdGet32(PcdSystemMemorySize) - 1) + // Update the global variable adr r2, mSystemMemoryEnd str r1, [r2] @@ -69,13 +51,12 @@ _SetupStackPosition: // r1 = SystemMemoryTop // Calculate Top of the Firmware Device - LoadConstantToReg (FixedPcdGet32(PcdFdBaseAddress), r2) - LoadConstantToReg (FixedPcdGet32(PcdFdSize), r3) - sub r3, r3, #1 + 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) - LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryUefiRegionSize), r4) + MOV32 (r4, FixedPcdGet32(PcdSystemMemoryUefiRegionSize)) // // Reserve the memory for the UEFI region (contain stacks on its top) @@ -106,9 +87,8 @@ _SetupAlignedStack: _SetupOverflowStack: // Case memory at the top of the address space. Ensure the top of the stack is EFI_PAGE_SIZE // aligned (4KB) - LoadConstantToReg (EFI_PAGE_MASK, r9) - and r9, r9, r1 - sub r1, r1, r9 + MOV32 (r9, ~EFI_PAGE_MASK & 0xFFFFFFFF) + and r1, r1, r9 _GetBaseUefiMemory: // Calculate the Base of the UEFI Memory @@ -117,22 +97,19 @@ _GetBaseUefiMemory: _GetStackBase: // r1 = The top of the Mpcore Stacks // Stack for the primary core = PrimaryCoreStack - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), r2) + MOV32 (r2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) sub r10, r1, r2 // Stack for the secondary core = Number of Cores - 1 - LoadConstantToReg (FixedPcdGet32(PcdCoreCount), r0) - sub r0, r0, #1 - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), r1) - mul r1, r1, r0 + MOV32 (r0, (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) - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), r2) - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), r3) + MOV32 (r2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) + MOV32 (r3, FixedPcdGet32(PcdCPUCoreSecondaryStackSize)) bl ASM_PFX(ArmPlatformStackSet) // Is it the Primary Core ? @@ -149,7 +126,7 @@ _PrepareArguments: // 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 + ldr r4, =ASM_PFX(CEntryPoint) // Jump to PrePiCore C code // r0 = MpId @@ -160,3 +137,4 @@ _PrepareArguments: _NeverReturn: b _NeverReturn +ASM_PFX(mSystemMemoryEnd): .8byte 0
Annotate functions with ASM_FUNC() so that they are emitted into separate sections. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S | 49 ++++++------------- ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S | 50 ++++++-------------- 2 files changed, 29 insertions(+), 70 deletions(-) -- 2.7.4 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel