Setup a proper frame pointer in efi32_pe_entry so that it's easier to
calculate offsets for arguments.
Signed-off-by: Arvind Sankar <[email protected]>
---
arch/x86/boot/compressed/head_64.S | 57 +++++++++++++++++++++---------
1 file changed, 40 insertions(+), 17 deletions(-)
diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 920daf62dac2..fabbd4c2e9f2 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -658,42 +658,65 @@ SYM_DATA(efi_is64, .byte 1)
.text
.code32
SYM_FUNC_START(efi32_pe_entry)
+/*
+ * efi_status_t efi32_pe_entry(efi_handle_t image_handle,
+ * efi_system_table_32_t *sys_table)
+ */
+
pushl %ebp
+ movl %esp, %ebp
+ pushl %eax // dummy push to allocate loaded_image
- pushl %ebx
+ pushl %ebx // save callee-save registers
pushl %edi
+
call verify_cpu // check for long mode support
- popl %edi
- popl %ebx
testl %eax, %eax
movl $0x80000003, %eax // EFI_UNSUPPORTED
- jnz 3f
+ jnz 2f
call 1f
-1: pop %ebp
- subl $1b, %ebp
+1: pop %ebx
+ subl $1b, %ebx
/* Get the loaded image protocol pointer from the image handle */
- subl $12, %esp // space for the loaded image pointer
- pushl %esp // pass its address
- leal loaded_image_proto(%ebp), %eax
+ leal -4(%ebp), %eax
+ pushl %eax // &loaded_image
+ leal loaded_image_proto(%ebx), %eax
pushl %eax // pass the GUID address
- pushl 28(%esp) // pass the image handle
+ pushl 8(%ebp) // pass the image handle
- movl 36(%esp), %eax // sys_table
+ /*
+ * Note the alignment of the stack frame.
+ * sys_table
+ * handle <-- 16-byte aligned on entry by ABI
+ * return address
+ * frame pointer
+ * loaded_image <-- local variable
+ * saved %ebx <-- 16-byte aligned here
+ * saved %edi
+ * &loaded_image
+ * &loaded_image_proto
+ * handle <-- 16-byte aligned for call to handle_protocol
+ */
+
+ movl 12(%ebp), %eax // sys_table
movl ST32_boottime(%eax), %eax // sys_table->boottime
call *BS32_handle_protocol(%eax) // sys_table->boottime->handle_protocol
- cmp $0, %eax
+ addl $12, %esp // restore argument space
+ testl %eax, %eax
jnz 2f
- movl 32(%esp), %ecx // image_handle
- movl 36(%esp), %edx // sys_table
- movl 12(%esp), %esi // loaded_image
+ movl 8(%ebp), %ecx // image_handle
+ movl 12(%ebp), %edx // sys_table
+ movl -4(%ebp), %esi // loaded_image
movl LI32_image_base(%esi), %esi // loaded_image->image_base
+ movl %ebx, %ebp // startup_32 for efi32_pe_stub_entry
jmp efi32_pe_stub_entry
-2: addl $24, %esp
-3: popl %ebp
+2: popl %edi // restore callee-save registers
+ popl %ebx
+ leave
ret
SYM_FUNC_END(efi32_pe_entry)
--
2.24.1
On Mon, 2 Mar 2020 at 00:04, Arvind Sankar <[email protected]> wrote:
>
> Setup a proper frame pointer in efi32_pe_entry so that it's easier to
> calculate offsets for arguments.
>
> Signed-off-by: Arvind Sankar <[email protected]>
> ---
> arch/x86/boot/compressed/head_64.S | 57 +++++++++++++++++++++---------
> 1 file changed, 40 insertions(+), 17 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
> index 920daf62dac2..fabbd4c2e9f2 100644
> --- a/arch/x86/boot/compressed/head_64.S
> +++ b/arch/x86/boot/compressed/head_64.S
> @@ -658,42 +658,65 @@ SYM_DATA(efi_is64, .byte 1)
> .text
> .code32
> SYM_FUNC_START(efi32_pe_entry)
> +/*
> + * efi_status_t efi32_pe_entry(efi_handle_t image_handle,
> + * efi_system_table_32_t *sys_table)
> + */
> +
> pushl %ebp
> + movl %esp, %ebp
> + pushl %eax // dummy push to allocate loaded_image
>
> - pushl %ebx
> + pushl %ebx // save callee-save registers
> pushl %edi
> +
> call verify_cpu // check for long mode support
> - popl %edi
> - popl %ebx
> testl %eax, %eax
> movl $0x80000003, %eax // EFI_UNSUPPORTED
> - jnz 3f
> + jnz 2f
>
> call 1f
> -1: pop %ebp
> - subl $1b, %ebp
> +1: pop %ebx
> + subl $1b, %ebx
>
> /* Get the loaded image protocol pointer from the image handle */
> - subl $12, %esp // space for the loaded image pointer
> - pushl %esp // pass its address
> - leal loaded_image_proto(%ebp), %eax
> + leal -4(%ebp), %eax
> + pushl %eax // &loaded_image
> + leal loaded_image_proto(%ebx), %eax
> pushl %eax // pass the GUID address
> - pushl 28(%esp) // pass the image handle
> + pushl 8(%ebp) // pass the image handle
>
> - movl 36(%esp), %eax // sys_table
> + /*
> + * Note the alignment of the stack frame.
> + * sys_table
> + * handle <-- 16-byte aligned on entry by ABI
> + * return address
> + * frame pointer
> + * loaded_image <-- local variable
> + * saved %ebx <-- 16-byte aligned here
> + * saved %edi
> + * &loaded_image
> + * &loaded_image_proto
> + * handle <-- 16-byte aligned for call to handle_protocol
> + */
> +
> + movl 12(%ebp), %eax // sys_table
> movl ST32_boottime(%eax), %eax // sys_table->boottime
> call *BS32_handle_protocol(%eax) // sys_table->boottime->handle_protocol
> - cmp $0, %eax
> + addl $12, %esp // restore argument space
> + testl %eax, %eax
> jnz 2f
>
> - movl 32(%esp), %ecx // image_handle
> - movl 36(%esp), %edx // sys_table
> - movl 12(%esp), %esi // loaded_image
> + movl 8(%ebp), %ecx // image_handle
> + movl 12(%ebp), %edx // sys_table
> + movl -4(%ebp), %esi // loaded_image
> movl LI32_image_base(%esi), %esi // loaded_image->image_base
> + movl %ebx, %ebp // startup_32 for efi32_pe_stub_entry
The code that follows efi32_pe_stub_entry still expects the runtime
displacement in %ebp, so we'll need to pass that in another way here.
> jmp efi32_pe_stub_entry
>
> -2: addl $24, %esp
> -3: popl %ebp
> +2: popl %edi // restore callee-save registers
> + popl %ebx
> + leave
> ret
> SYM_FUNC_END(efi32_pe_entry)
>
> --
> 2.24.1
>
On Mon, Mar 02, 2020 at 08:49:17AM +0100, Ard Biesheuvel wrote:
> On Mon, 2 Mar 2020 at 00:04, Arvind Sankar <[email protected]> wrote:
...
> > call 1f
> > -1: pop %ebp
> > - subl $1b, %ebp
> > +1: pop %ebx
> > + subl $1b, %ebx
...
> >
> > + movl %ebx, %ebp // startup_32 for efi32_pe_stub_entry
>
> The code that follows efi32_pe_stub_entry still expects the runtime
> displacement in %ebp, so we'll need to pass that in another way here.
>
> > jmp efi32_pe_stub_entry
Didn't follow -- what do you mean by runtime displacement?
efi32_pe_stub_entry expects the runtime address of startup_32 to be in
%ebp, but with the changes for keeping the frame pointer in %ebp, I
changed the runtime address to be in %ebx instead. Hence I added that
movl %ebx, %ebp to put it in %ebp just before calling efi32_pe_stub_entry.
That should be fine, no?
On Mon, 2 Mar 2020 at 17:54, Arvind Sankar <[email protected]> wrote:
>
> On Mon, Mar 02, 2020 at 08:49:17AM +0100, Ard Biesheuvel wrote:
> > On Mon, 2 Mar 2020 at 00:04, Arvind Sankar <[email protected]> wrote:
> ...
> > > call 1f
> > > -1: pop %ebp
> > > - subl $1b, %ebp
> > > +1: pop %ebx
> > > + subl $1b, %ebx
> ...
> > >
> > > + movl %ebx, %ebp // startup_32 for efi32_pe_stub_entry
> >
> > The code that follows efi32_pe_stub_entry still expects the runtime
> > displacement in %ebp, so we'll need to pass that in another way here.
> >
> > > jmp efi32_pe_stub_entry
>
> Didn't follow -- what do you mean by runtime displacement?
>
> efi32_pe_stub_entry expects the runtime address of startup_32 to be in
> %ebp, but with the changes for keeping the frame pointer in %ebp, I
> changed the runtime address to be in %ebx instead. Hence I added that
> movl %ebx, %ebp to put it in %ebp just before calling efi32_pe_stub_entry.
> That should be fine, no?
But how does that work with:
SYM_INNER_LABEL(efi32_pe_stub_entry, SYM_L_LOCAL)
movl %ecx, efi32_boot_args(%ebp)
movl %edx, efi32_boot_args+4(%ebp)
movb $0, efi_is64(%ebp)
?
On Mon, Mar 02, 2020 at 05:57:04PM +0100, Ard Biesheuvel wrote:
> On Mon, 2 Mar 2020 at 17:54, Arvind Sankar <[email protected]> wrote:
> >
> > On Mon, Mar 02, 2020 at 08:49:17AM +0100, Ard Biesheuvel wrote:
> > > On Mon, 2 Mar 2020 at 00:04, Arvind Sankar <[email protected]> wrote:
> > ...
> > > > call 1f
> > > > -1: pop %ebp
> > > > - subl $1b, %ebp
> > > > +1: pop %ebx
> > > > + subl $1b, %ebx
> > ...
> > > >
> > > > + movl %ebx, %ebp // startup_32 for efi32_pe_stub_entry
> > >
> > > The code that follows efi32_pe_stub_entry still expects the runtime
> > > displacement in %ebp, so we'll need to pass that in another way here.
> > >
> > > > jmp efi32_pe_stub_entry
> >
> > Didn't follow -- what do you mean by runtime displacement?
> >
> > efi32_pe_stub_entry expects the runtime address of startup_32 to be in
> > %ebp, but with the changes for keeping the frame pointer in %ebp, I
> > changed the runtime address to be in %ebx instead. Hence I added that
> > movl %ebx, %ebp to put it in %ebp just before calling efi32_pe_stub_entry.
> > That should be fine, no?
>
> But how does that work with:
>
> SYM_INNER_LABEL(efi32_pe_stub_entry, SYM_L_LOCAL)
> movl %ecx, efi32_boot_args(%ebp)
> movl %edx, efi32_boot_args+4(%ebp)
> movb $0, efi_is64(%ebp)
>
>
> ?
Why wouldn't it work? Before this change, efi32_pe_entry set %ebp to
startup_32 (via the call/pop/sub sequence), so efi32_pe_stub_entry was
entered with %ebp == startup_32.
After this change, the call/pop/sub sequence puts startup_32 into %ebx,
and then I copy it into %ebp just before branching to efi32_pe_stub_entry.
So everything should continue to work the same way as before?
On Mon, 2 Mar 2020 at 18:02, Arvind Sankar <[email protected]> wrote:
>
> On Mon, Mar 02, 2020 at 05:57:04PM +0100, Ard Biesheuvel wrote:
> > On Mon, 2 Mar 2020 at 17:54, Arvind Sankar <[email protected]> wrote:
> > >
> > > On Mon, Mar 02, 2020 at 08:49:17AM +0100, Ard Biesheuvel wrote:
> > > > On Mon, 2 Mar 2020 at 00:04, Arvind Sankar <[email protected]> wrote:
> > > ...
> > > > > call 1f
> > > > > -1: pop %ebp
> > > > > - subl $1b, %ebp
> > > > > +1: pop %ebx
> > > > > + subl $1b, %ebx
> > > ...
> > > > >
> > > > > + movl %ebx, %ebp // startup_32 for efi32_pe_stub_entry
> > > >
> > > > The code that follows efi32_pe_stub_entry still expects the runtime
> > > > displacement in %ebp, so we'll need to pass that in another way here.
> > > >
> > > > > jmp efi32_pe_stub_entry
> > >
> > > Didn't follow -- what do you mean by runtime displacement?
> > >
> > > efi32_pe_stub_entry expects the runtime address of startup_32 to be in
> > > %ebp, but with the changes for keeping the frame pointer in %ebp, I
> > > changed the runtime address to be in %ebx instead. Hence I added that
> > > movl %ebx, %ebp to put it in %ebp just before calling efi32_pe_stub_entry.
> > > That should be fine, no?
> >
> > But how does that work with:
> >
> > SYM_INNER_LABEL(efi32_pe_stub_entry, SYM_L_LOCAL)
> > movl %ecx, efi32_boot_args(%ebp)
> > movl %edx, efi32_boot_args+4(%ebp)
> > movb $0, efi_is64(%ebp)
> >
> >
> > ?
>
> Why wouldn't it work? Before this change, efi32_pe_entry set %ebp to
> startup_32 (via the call/pop/sub sequence), so efi32_pe_stub_entry was
> entered with %ebp == startup_32.
>
> After this change, the call/pop/sub sequence puts startup_32 into %ebx,
> and then I copy it into %ebp just before branching to efi32_pe_stub_entry.
> So everything should continue to work the same way as before?
Ah ok, so the sequence
call 1f
1: pop %ebp
subl $1b, %ebp
happens to produce the value of startup_32, which is obvious now that
I think of it, but it wasn't before.