ArmVirtQemuKernel uses ArmVirtPkg/PrePi/Arm/ModuleEntryPoint.S which is not affected by this bug.
as it turns out, only BeagleBoardPkg uses the generic PrePi code. On Wed, Aug 31, 2016 at 11:14 AM, Ard Biesheuvel <[email protected]> wrote: > On 31 August 2016 at 05:33, Michael Zimmermann <[email protected]> > 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 < > [email protected]> > > 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 <[email protected]> > >> > >> > Contributed-under: TianoCore Contribution Agreement 1.0 > >> > Signed-off-by: Ard Biesheuvel <[email protected]> > >> > --- > >> > 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 > >> [email protected] > >> https://lists.01.org/mailman/listinfo/edk2-devel > > > > > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
