public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 1/2] ArmVirtPkg/PrePi: use correct callee saved regs
@ 2016-08-05 14:53 Ard Biesheuvel
  2016-08-05 14:53 ` [PATCH 2/2] ArmPlatformPkg/PrePi: " Ard Biesheuvel
  2016-08-05 18:31 ` [PATCH 1/2] ArmVirtPkg/PrePi: " Laszlo Ersek
  0 siblings, 2 replies; 3+ messages in thread
From: Ard Biesheuvel @ 2016-08-05 14:53 UTC (permalink / raw)
  To: edk2-devel; +Cc: leif.lindholm, lersek, Ard Biesheuvel

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)
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH 2/2] ArmPlatformPkg/PrePi: use correct callee saved regs
  2016-08-05 14:53 [PATCH 1/2] ArmVirtPkg/PrePi: use correct callee saved regs Ard Biesheuvel
@ 2016-08-05 14:53 ` Ard Biesheuvel
  2016-08-05 18:31 ` [PATCH 1/2] ArmVirtPkg/PrePi: " Laszlo Ersek
  1 sibling, 0 replies; 3+ messages in thread
From: Ard Biesheuvel @ 2016-08-05 14:53 UTC (permalink / raw)
  To: edk2-devel; +Cc: leif.lindholm, lersek, Ard Biesheuvel

The AARCH64 version of the PrePi code 'preserves' 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>
---
 ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S | 32 ++++++++++----------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S
index 9538c70a237c..e939d2c5aa5e 100644
--- a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S
+++ b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S
@@ -36,7 +36,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
 
 _SetSVCMode:
 // Check if we can install the stack at the top of the System Memory or if we need
@@ -88,55 +88,55 @@ _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
+  LoadConstantToReg (EFI_PAGE_MASK, x21)
+  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   x1, x10
+  // x22 = The base of the MpCore Stacks (primary stack & secondary stacks)
+  mov   x0, x22
+  mov   x1, x20
   //ArmPlatformStackSet(StackBase, MpId, PrimaryStackSize, SecondaryStackSize)
   LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), x2)
   LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), x3)
   bl    ASM_PFX(ArmPlatformStackSet)
 
   // Is it the Primary Core ?
-  mov   x0, x10
+  mov   x0, x20
   bl    ASM_PFX(ArmPlatformIsPrimaryCore)
   cmp   x0, #1
   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)
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/2] ArmVirtPkg/PrePi: use correct callee saved regs
  2016-08-05 14:53 [PATCH 1/2] ArmVirtPkg/PrePi: use correct callee saved regs Ard Biesheuvel
  2016-08-05 14:53 ` [PATCH 2/2] ArmPlatformPkg/PrePi: " Ard Biesheuvel
@ 2016-08-05 18:31 ` Laszlo Ersek
  1 sibling, 0 replies; 3+ messages in thread
From: Laszlo Ersek @ 2016-08-05 18:31 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel; +Cc: leif.lindholm

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>


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-08-05 18:31 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-05 14:53 [PATCH 1/2] ArmVirtPkg/PrePi: use correct callee saved regs Ard Biesheuvel
2016-08-05 14:53 ` [PATCH 2/2] ArmPlatformPkg/PrePi: " Ard Biesheuvel
2016-08-05 18:31 ` [PATCH 1/2] ArmVirtPkg/PrePi: " Laszlo Ersek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox