| Message ID | 1470408838-1020-1-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | Accepted |
| Commit | 298f83612375bda69e8a7230dfea036e85eda553 |
| Headers | show |
On 08/05/16 16:53, Ard Biesheuvel wrote: > Both the ARM and the AARCH64 versions of the PrePi code (shared between > ArmVirtQemuKernel and ArmVirtXen) 'preserve' values across a function > call using registers that are not in fact callee saved. So fix that. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S | 24 ++++++++++---------- > ArmVirtPkg/PrePi/Arm/ModuleEntryPoint.S | 10 ++++---- > 2 files changed, 17 insertions(+), 17 deletions(-) > > diff --git a/ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S b/ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S > index 68049d5df2bf..e61f5df12e89 100644 > --- a/ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S > +++ b/ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S > @@ -71,7 +71,7 @@ ASM_PFX(_ModuleEntryPoint): > // Get ID of this CPU in Multicore system > bl ASM_PFX(ArmReadMpidr) > // Keep a copy of the MpId register value > - mov x10, x0 > + mov x20, x0 > > // 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 > @@ -113,39 +113,39 @@ _SetupStack: > // Because the 'push' instruction is equivalent to 'stmdb' (decrement before), we need to increment > // one to the top of the stack. We check if incrementing one does not overflow (case of DRAM at the > // top of the memory space) > - adds x11, x1, #1 > + adds x21, x1, #1 > b.cs _SetupOverflowStack > > _SetupAlignedStack: > - mov x1, x11 > + mov x1, x21 > b _GetBaseUefiMemory > > _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 x21, x21, x1 > + sub x1, x1, x21 > > _GetBaseUefiMemory: > // Calculate the Base of the UEFI Memory > - sub x11, x1, x4 > + sub x21, x1, x4 > > _GetStackBase: > // r1 = The top of the Mpcore Stacks > // Stack for the primary core = PrimaryCoreStack > LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), x2) > - sub x12, x1, x2 > + sub x22, 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 > - sub x12, x12, x1 > + sub x22, x22, x1 > > // x12 = The base of the MpCore Stacks (primary stack & secondary stacks) > - mov x0, x12 > + mov x0, x22 > mov x1, x10 > //ArmPlatformStackSet(StackBase, MpId, PrimaryStackSize, SecondaryStackSize) > LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), x2) > @@ -159,9 +159,9 @@ _GetStackBase: > bne _PrepareArguments > > _PrepareArguments: > - mov x0, x10 > - mov x1, x11 > - mov x2, x12 > + mov x0, x20 > + mov x1, x21 > + mov x2, x22 > > // Move sec startup address into a data register > // Ensure we're jumping to FV version of the code (not boot remapped alias) > diff --git a/ArmVirtPkg/PrePi/Arm/ModuleEntryPoint.S b/ArmVirtPkg/PrePi/Arm/ModuleEntryPoint.S > index 441db36857de..3215c7d55876 100644 > --- a/ArmVirtPkg/PrePi/Arm/ModuleEntryPoint.S > +++ b/ArmVirtPkg/PrePi/Arm/ModuleEntryPoint.S > @@ -154,17 +154,17 @@ _GetStackBase: > // r1 = The top of the Mpcore Stacks > // Stack for the primary core = PrimaryCoreStack > LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), r2) > - sub r12, r1, r2 > + sub r9, 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 > - sub r12, r12, r1 > + sub r9, r9, r1 > > - // r12 = The base of the MpCore Stacks (primary stack & secondary stacks) > - mov r0, r12 > + // r9 = The base of the MpCore Stacks (primary stack & secondary stacks) > + mov r0, r9 > mov r1, r10 > //ArmPlatformStackSet(StackBase, MpId, PrimaryStackSize, SecondaryStackSize) > LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), r2) > @@ -180,7 +180,7 @@ _GetStackBase: > _PrepareArguments: > mov r0, r10 > mov r1, r11 > - mov r2, r12 > + mov r2, r9 > > // Move sec startup address into a data register > // Ensure we're jumping to FV version of the code (not boot remapped alias) > Callee saved, caller saved... Bah! :) Acked-by: Laszlo Ersek <lersek@redhat.com> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S b/ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S index 68049d5df2bf..e61f5df12e89 100644 --- a/ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S +++ b/ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S @@ -71,7 +71,7 @@ ASM_PFX(_ModuleEntryPoint): // Get ID of this CPU in Multicore system bl ASM_PFX(ArmReadMpidr) // Keep a copy of the MpId register value - mov x10, x0 + mov x20, x0 // 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 @@ -113,39 +113,39 @@ _SetupStack: // Because the 'push' instruction is equivalent to 'stmdb' (decrement before), we need to increment // one to the top of the stack. We check if incrementing one does not overflow (case of DRAM at the // top of the memory space) - adds x11, x1, #1 + adds x21, x1, #1 b.cs _SetupOverflowStack _SetupAlignedStack: - mov x1, x11 + mov x1, x21 b _GetBaseUefiMemory _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 x21, x21, x1 + sub x1, x1, x21 _GetBaseUefiMemory: // Calculate the Base of the UEFI Memory - sub x11, x1, x4 + sub x21, x1, x4 _GetStackBase: // r1 = The top of the Mpcore Stacks // Stack for the primary core = PrimaryCoreStack LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), x2) - sub x12, x1, x2 + sub x22, 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 - sub x12, x12, x1 + sub x22, x22, x1 // x12 = The base of the MpCore Stacks (primary stack & secondary stacks) - mov x0, x12 + mov x0, x22 mov x1, x10 //ArmPlatformStackSet(StackBase, MpId, PrimaryStackSize, SecondaryStackSize) LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), x2) @@ -159,9 +159,9 @@ _GetStackBase: bne _PrepareArguments _PrepareArguments: - mov x0, x10 - mov x1, x11 - mov x2, x12 + mov x0, x20 + mov x1, x21 + mov x2, x22 // Move sec startup address into a data register // Ensure we're jumping to FV version of the code (not boot remapped alias) diff --git a/ArmVirtPkg/PrePi/Arm/ModuleEntryPoint.S b/ArmVirtPkg/PrePi/Arm/ModuleEntryPoint.S index 441db36857de..3215c7d55876 100644 --- a/ArmVirtPkg/PrePi/Arm/ModuleEntryPoint.S +++ b/ArmVirtPkg/PrePi/Arm/ModuleEntryPoint.S @@ -154,17 +154,17 @@ _GetStackBase: // r1 = The top of the Mpcore Stacks // Stack for the primary core = PrimaryCoreStack LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), r2) - sub r12, r1, r2 + sub r9, 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 - sub r12, r12, r1 + sub r9, r9, r1 - // r12 = The base of the MpCore Stacks (primary stack & secondary stacks) - mov r0, r12 + // r9 = The base of the MpCore Stacks (primary stack & secondary stacks) + mov r0, r9 mov r1, r10 //ArmPlatformStackSet(StackBase, MpId, PrimaryStackSize, SecondaryStackSize) LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), r2) @@ -180,7 +180,7 @@ _GetStackBase: _PrepareArguments: mov r0, r10 mov r1, r11 - mov r2, r12 + mov r2, r9 // Move sec startup address into a data register // Ensure we're jumping to FV version of the code (not boot remapped alias)
Both the ARM and the AARCH64 versions of the PrePi code (shared between ArmVirtQemuKernel and ArmVirtXen) 'preserve' values across a function call using registers that are not in fact callee saved. So fix that. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S | 24 ++++++++++---------- ArmVirtPkg/PrePi/Arm/ModuleEntryPoint.S | 10 ++++---- 2 files changed, 17 insertions(+), 17 deletions(-) -- 2.7.4 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel