Hi Ricardo,
Thanks for this, let me go through the idea I had. Please let me know if
I am missing something.
On 21/06/2022 23:32, Ricardo Koller wrote:
On Fri, May 06, 2022 at 09:56:01PM +0100, Nikos Nikoleris wrote:
arm/efi/crt0-efi-aarch64.S defines the header and the handover
sequence from EFI to a efi_main. This change includes the whole file
in arm/cstart64.S when we compile with EFI support.
In addition, we change the handover code in arm/efi/crt0-efi-aarch64.S
to align the stack pointer. This alignment is necessary because we
make assumptions about cpu0's stack alignment and most importantly we
place its thread_info at the bottom of this stack.
Signed-off-by: Nikos Nikoleris <nikos.nikoleris@xxxxxxx>
---
arm/cstart64.S | 6 ++++++
arm/efi/crt0-efi-aarch64.S | 17 +++++++++++++++--
2 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/arm/cstart64.S b/arm/cstart64.S
index 55b41ea..08cf02f 100644
--- a/arm/cstart64.S
+++ b/arm/cstart64.S
@@ -15,6 +15,10 @@
#include <asm/thread_info.h>
#include <asm/sysreg.h>
+#ifdef CONFIG_EFI
+#include "efi/crt0-efi-aarch64.S"
+#else
+
.macro zero_range, tmp1, tmp2
9998: cmp \tmp1, \tmp2
b.eq 9997f
@@ -107,6 +111,8 @@ start:
bl exit
b halt
+#endif
+
.text
/*
diff --git a/arm/efi/crt0-efi-aarch64.S b/arm/efi/crt0-efi-aarch64.S
index d50e78d..11a062d 100644
--- a/arm/efi/crt0-efi-aarch64.S
+++ b/arm/efi/crt0-efi-aarch64.S
@@ -111,10 +111,19 @@ section_table:
.align 12
_start:
- stp x29, x30, [sp, #-32]!
+ stp x29, x30, [sp, #-16]!
Is this and the "ldp x29, x30, [sp], #16" change below needed?
why is #-32 not good?
The stack is full-descending. Here we make space for x29 and x30 in the
stack (16bytes) and save the two registers
+
+ // Align sp; this is necessary due to way we store cpu0's thread_info
/* */ comment style
ack
mov x29, sp
+ and x29, x29, #THREAD_MASK
+ mov x30, sp
+ mov sp, x29
+ str x30, [sp, #-32]!
+
Here we're making space in the stack for the old sp (x30), x0 and x1 but
we have to also ensure that the sp is aligned (32bytes). The we store x30.
(As a side note, I could also change this to
+ str x30, [sp, #-16]!
and change the next stp to do pre-incrementing mode. This might make
things simpler.)
+ mov x29, sp
stp x0, x1, [sp, #16]
+
Here, we use the space we made before to store x0 and x1.
I think, the stack now should look like:
| ... |
| x30 |
| x29 |
| x1 |
| x0 |
| pad |
sp -> | old_sp |
mov x2, x0
mov x3, x1
adr x0, ImageBase
@@ -126,5 +135,9 @@ _start:
ldp x0, x1, [sp, #16]
bl efi_main
-0: ldp x29, x30, [sp], #32
+ // Restore sp
/* */ comment style
ack
+ ldr x30, [sp]
I think this should have been:
+ ldr x30, [sp], #32
Restore x30 from the current sp and free up space in the stack (all
32bytes).
I'm not able to understand this. Is this ldr restoring the value pushed
with "str x30, [sp, #-32]!" above? in that case, shouldn't this be at
[sp - 32]? But, given that this code is unreachable when efi_main is
called, do you even need to restore the sp?
+ mov sp, x30
+
+0: ldp x29, x30, [sp], #16
Then, this restores x29 and x30 and frees up the the corresponding space
in the stack.
I am not sure we shouldn't get to this point and I wanted to properly
save and restore the register state. I haven't really found what's the
right/best way to exit from an EFI app and I wanted to allow for
graceful return from this point. But I am happy to change all this.
Thanks,
Nikos
ret
--
2.25.1
