* [PATCH 0/5] efi/x86 cleanups and one bugfix
@ 2020-03-01 23:04 Arvind Sankar
2020-03-01 23:04 ` [PATCH 1/5] efi/x86: Annotate the LOADED_IMAGE_PROTOCOL_GUID with SYM_DATA Arvind Sankar
` (5 more replies)
0 siblings, 6 replies; 12+ messages in thread
From: Arvind Sankar @ 2020-03-01 23:04 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel
First 3 patches are misc. beautifications to the new compat PE entry
code.
Next patch stops EFI stub using code32_start field to communicate the
address of startup_32, instead returning it directly to efi_stub_entry.
Last patch is a bugfix for x86/boot/head code to use unsigned
comparisons on addresses rather than signed.
Based on tip:efi/core
Arvind Sankar (5):
efi/x86: Annotate the LOADED_IMAGE_PROTOCOL_GUID with SYM_DATA
efi/x86: Respect 32-bit ABI in efi32_pe_entry
efi/x86: Make efi32_pe_entry more readable
efi/x86: Avoid using code32_start
x86/boot: Use unsigned comparison for addresses
arch/x86/boot/compressed/head_32.S | 5 +-
arch/x86/boot/compressed/head_64.S | 70 ++++++++++++++++++-------
arch/x86/kernel/asm-offsets.c | 1 -
drivers/firmware/efi/libstub/x86-stub.c | 10 ++--
4 files changed, 57 insertions(+), 29 deletions(-)
--
2.24.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/5] efi/x86: Annotate the LOADED_IMAGE_PROTOCOL_GUID with SYM_DATA
2020-03-01 23:04 [PATCH 0/5] efi/x86 cleanups and one bugfix Arvind Sankar
@ 2020-03-01 23:04 ` Arvind Sankar
2020-03-01 23:04 ` [PATCH 2/5] efi/x86: Respect 32-bit ABI in efi32_pe_entry Arvind Sankar
` (4 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Arvind Sankar @ 2020-03-01 23:04 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel
Use SYM_DATA* macro to annotate this constant, and explicitly align it
to 4-byte boundary. Use lower-case for hexadecimal data.
Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
arch/x86/boot/compressed/head_64.S | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index fcf8feaa57ea..8105e8348607 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -672,7 +672,7 @@ SYM_FUNC_START(efi32_pe_entry)
/* 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 4f(%ebp), %eax
+ leal loaded_image_proto(%ebp), %eax
pushl %eax // pass the GUID address
pushl 28(%esp) // pass the image handle
@@ -695,9 +695,12 @@ SYM_FUNC_END(efi32_pe_entry)
.section ".rodata"
/* EFI loaded image protocol GUID */
-4: .long 0x5B1B31A1
+ .balign 4
+SYM_DATA_START_LOCAL(loaded_image_proto)
+ .long 0x5b1b31a1
.word 0x9562, 0x11d2
- .byte 0x8E, 0x3F, 0x00, 0xA0, 0xC9, 0x69, 0x72, 0x3B
+ .byte 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b
+SYM_DATA_END(loaded_image_proto)
#endif
/*
--
2.24.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/5] efi/x86: Respect 32-bit ABI in efi32_pe_entry
2020-03-01 23:04 [PATCH 0/5] efi/x86 cleanups and one bugfix Arvind Sankar
2020-03-01 23:04 ` [PATCH 1/5] efi/x86: Annotate the LOADED_IMAGE_PROTOCOL_GUID with SYM_DATA Arvind Sankar
@ 2020-03-01 23:04 ` Arvind Sankar
2020-03-01 23:04 ` [PATCH 3/5] efi/x86: Make efi32_pe_entry more readable Arvind Sankar
` (3 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Arvind Sankar @ 2020-03-01 23:04 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel
verify_cpu clobbers BX and DI. In case we have to return error, we need
to preserve them to respect 32-bit calling convention.
Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
arch/x86/boot/compressed/head_64.S | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 8105e8348607..920daf62dac2 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -660,7 +660,11 @@ SYM_DATA(efi_is64, .byte 1)
SYM_FUNC_START(efi32_pe_entry)
pushl %ebp
+ pushl %ebx
+ pushl %edi
call verify_cpu // check for long mode support
+ popl %edi
+ popl %ebx
testl %eax, %eax
movl $0x80000003, %eax // EFI_UNSUPPORTED
jnz 3f
--
2.24.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/5] efi/x86: Make efi32_pe_entry more readable
2020-03-01 23:04 [PATCH 0/5] efi/x86 cleanups and one bugfix Arvind Sankar
2020-03-01 23:04 ` [PATCH 1/5] efi/x86: Annotate the LOADED_IMAGE_PROTOCOL_GUID with SYM_DATA Arvind Sankar
2020-03-01 23:04 ` [PATCH 2/5] efi/x86: Respect 32-bit ABI in efi32_pe_entry Arvind Sankar
@ 2020-03-01 23:04 ` Arvind Sankar
2020-03-02 7:49 ` Ard Biesheuvel
2020-03-01 23:04 ` [PATCH 4/5] efi/x86: Avoid using code32_start Arvind Sankar
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Arvind Sankar @ 2020-03-01 23:04 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel
Setup a proper frame pointer in efi32_pe_entry so that it's easier to
calculate offsets for arguments.
Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 4/5] efi/x86: Avoid using code32_start
2020-03-01 23:04 [PATCH 0/5] efi/x86 cleanups and one bugfix Arvind Sankar
` (2 preceding siblings ...)
2020-03-01 23:04 ` [PATCH 3/5] efi/x86: Make efi32_pe_entry more readable Arvind Sankar
@ 2020-03-01 23:04 ` Arvind Sankar
2020-03-01 23:04 ` [PATCH 5/5] x86/boot: Use unsigned comparison for addresses Arvind Sankar
2020-03-02 7:50 ` [PATCH 0/5] efi/x86 cleanups and one bugfix Ard Biesheuvel
5 siblings, 0 replies; 12+ messages in thread
From: Arvind Sankar @ 2020-03-01 23:04 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel
code32_start is meant for 16-bit real-mode bootloaders to inform the
kernel where the 32-bit protected mode code starts. Nothing in the
protected mode kernel except the EFI stub uses it.
efi_main currently returns boot_params, with code32_start set inside it
to tell efi_stub_entry where startup_32 is located. Since it was invoked
by efi_stub_entry in the first place, boot_params is already known.
Return the address of startup_32 instead.
This will allow a 64-bit kernel to live above 4Gb, for example, and it's
cleaner.
Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
arch/x86/boot/compressed/head_32.S | 3 +--
arch/x86/boot/compressed/head_64.S | 4 ++--
arch/x86/kernel/asm-offsets.c | 1 -
drivers/firmware/efi/libstub/x86-stub.c | 10 +++++-----
4 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
index 2f8138b71ea9..e013bdc1237b 100644
--- a/arch/x86/boot/compressed/head_32.S
+++ b/arch/x86/boot/compressed/head_32.S
@@ -156,9 +156,8 @@ SYM_FUNC_END(startup_32)
SYM_FUNC_START(efi32_stub_entry)
SYM_FUNC_START_ALIAS(efi_stub_entry)
add $0x4, %esp
+ movl 8(%esp), %esi /* save boot_params pointer */
call efi_main
- movl %eax, %esi
- movl BP_code32_start(%esi), %eax
leal startup_32(%eax), %eax
jmp *%eax
SYM_FUNC_END(efi32_stub_entry)
diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index fabbd4c2e9f2..6a4ff919008c 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -472,9 +472,9 @@ SYM_CODE_END(startup_64)
SYM_FUNC_START(efi64_stub_entry)
SYM_FUNC_START_ALIAS(efi_stub_entry)
and $~0xf, %rsp /* realign the stack */
+ movq %rdx, %rbx /* save boot_params pointer */
call efi_main
- movq %rax,%rsi
- movl BP_code32_start(%esi), %eax
+ movq %rbx,%rsi
leaq startup_64(%rax), %rax
jmp *%rax
SYM_FUNC_END(efi64_stub_entry)
diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.c
index 5c7ee3df4d0b..3ca07ad552ae 100644
--- a/arch/x86/kernel/asm-offsets.c
+++ b/arch/x86/kernel/asm-offsets.c
@@ -88,7 +88,6 @@ static void __used common(void)
OFFSET(BP_kernel_alignment, boot_params, hdr.kernel_alignment);
OFFSET(BP_init_size, boot_params, hdr.init_size);
OFFSET(BP_pref_address, boot_params, hdr.pref_address);
- OFFSET(BP_code32_start, boot_params, hdr.code32_start);
BLANK();
DEFINE(PTREGS_SIZE, sizeof(struct pt_regs));
diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index 9db98839d7b4..7f3e97c2aad3 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -703,10 +703,11 @@ static efi_status_t exit_boot(struct boot_params *boot_params, void *handle)
}
/*
- * On success we return a pointer to a boot_params structure, and NULL
- * on failure.
+ * On success, we return the address of startup_32, which has potentially been
+ * relocated by efi_relocate_kernel.
+ * On failure, we exit to the firmware via efi_exit instead of returning.
*/
-struct boot_params *efi_main(efi_handle_t handle,
+unsigned long efi_main(efi_handle_t handle,
efi_system_table_t *sys_table_arg,
struct boot_params *boot_params)
{
@@ -736,7 +737,6 @@ struct boot_params *efi_main(efi_handle_t handle,
goto fail;
}
}
- hdr->code32_start = (u32)bzimage_addr;
/*
* efi_pe_entry() may have been called before efi_main(), in which
@@ -799,7 +799,7 @@ struct boot_params *efi_main(efi_handle_t handle,
goto fail;
}
- return boot_params;
+ return bzimage_addr;
fail:
efi_printk("efi_main() failed!\n");
--
2.24.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 5/5] x86/boot: Use unsigned comparison for addresses
2020-03-01 23:04 [PATCH 0/5] efi/x86 cleanups and one bugfix Arvind Sankar
` (3 preceding siblings ...)
2020-03-01 23:04 ` [PATCH 4/5] efi/x86: Avoid using code32_start Arvind Sankar
@ 2020-03-01 23:04 ` Arvind Sankar
2020-03-02 7:50 ` [PATCH 0/5] efi/x86 cleanups and one bugfix Ard Biesheuvel
5 siblings, 0 replies; 12+ messages in thread
From: Arvind Sankar @ 2020-03-01 23:04 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel
The load address is compared with LOAD_PHYSICAL_ADDR using a signed
comparison currently (using jge instruction).
When loading a 64-bit kernel using the new efi32_pe_entry point added by
commit 97aa276579b2 ("efi/x86: Add true mixed mode entry point into
.compat section") using qemu with -m 3072, the firmware actually loads
us above 2Gb, resulting in a very early crash.
Use jae instruction to perform unsigned comparison instead, as physical
addresses should be considered as unsigned.
Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
arch/x86/boot/compressed/head_32.S | 2 +-
arch/x86/boot/compressed/head_64.S | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
index e013bdc1237b..46bbe7ab4adf 100644
--- a/arch/x86/boot/compressed/head_32.S
+++ b/arch/x86/boot/compressed/head_32.S
@@ -105,7 +105,7 @@ SYM_FUNC_START(startup_32)
notl %eax
andl %eax, %ebx
cmpl $LOAD_PHYSICAL_ADDR, %ebx
- jge 1f
+ jae 1f
#endif
movl $LOAD_PHYSICAL_ADDR, %ebx
1:
diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 6a4ff919008c..5d8338a693ce 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -105,7 +105,7 @@ SYM_FUNC_START(startup_32)
notl %eax
andl %eax, %ebx
cmpl $LOAD_PHYSICAL_ADDR, %ebx
- jge 1f
+ jae 1f
#endif
movl $LOAD_PHYSICAL_ADDR, %ebx
1:
@@ -305,7 +305,7 @@ SYM_CODE_START(startup_64)
notq %rax
andq %rax, %rbp
cmpq $LOAD_PHYSICAL_ADDR, %rbp
- jge 1f
+ jae 1f
#endif
movq $LOAD_PHYSICAL_ADDR, %rbp
1:
--
2.24.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 3/5] efi/x86: Make efi32_pe_entry more readable
2020-03-01 23:04 ` [PATCH 3/5] efi/x86: Make efi32_pe_entry more readable Arvind Sankar
@ 2020-03-02 7:49 ` Ard Biesheuvel
2020-03-02 16:54 ` Arvind Sankar
0 siblings, 1 reply; 12+ messages in thread
From: Ard Biesheuvel @ 2020-03-02 7:49 UTC (permalink / raw)
To: Arvind Sankar
Cc: linux-efi, the arch/x86 maintainers, Linux Kernel Mailing List
On Mon, 2 Mar 2020 at 00:04, Arvind Sankar <nivedita@alum.mit.edu> 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 <nivedita@alum.mit.edu>
> ---
> 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
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/5] efi/x86 cleanups and one bugfix
2020-03-01 23:04 [PATCH 0/5] efi/x86 cleanups and one bugfix Arvind Sankar
` (4 preceding siblings ...)
2020-03-01 23:04 ` [PATCH 5/5] x86/boot: Use unsigned comparison for addresses Arvind Sankar
@ 2020-03-02 7:50 ` Ard Biesheuvel
5 siblings, 0 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2020-03-02 7:50 UTC (permalink / raw)
To: Arvind Sankar
Cc: linux-efi, the arch/x86 maintainers, Linux Kernel Mailing List
On Mon, 2 Mar 2020 at 00:04, Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> First 3 patches are misc. beautifications to the new compat PE entry
> code.
>
> Next patch stops EFI stub using code32_start field to communicate the
> address of startup_32, instead returning it directly to efi_stub_entry.
>
> Last patch is a bugfix for x86/boot/head code to use unsigned
> comparisons on addresses rather than signed.
>
> Based on tip:efi/core
>
> Arvind Sankar (5):
> efi/x86: Annotate the LOADED_IMAGE_PROTOCOL_GUID with SYM_DATA
> efi/x86: Respect 32-bit ABI in efi32_pe_entry
> efi/x86: Make efi32_pe_entry more readable
> efi/x86: Avoid using code32_start
> x86/boot: Use unsigned comparison for addresses
>
Thanks Arvind. This looks really good, as usual. I had one question,
the rest looks good to go.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/5] efi/x86: Make efi32_pe_entry more readable
2020-03-02 7:49 ` Ard Biesheuvel
@ 2020-03-02 16:54 ` Arvind Sankar
2020-03-02 16:57 ` Ard Biesheuvel
0 siblings, 1 reply; 12+ messages in thread
From: Arvind Sankar @ 2020-03-02 16:54 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Arvind Sankar, linux-efi, the arch/x86 maintainers,
Linux Kernel Mailing List
On Mon, Mar 02, 2020 at 08:49:17AM +0100, Ard Biesheuvel wrote:
> On Mon, 2 Mar 2020 at 00:04, Arvind Sankar <nivedita@alum.mit.edu> 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?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/5] efi/x86: Make efi32_pe_entry more readable
2020-03-02 16:54 ` Arvind Sankar
@ 2020-03-02 16:57 ` Ard Biesheuvel
2020-03-02 17:02 ` Arvind Sankar
0 siblings, 1 reply; 12+ messages in thread
From: Ard Biesheuvel @ 2020-03-02 16:57 UTC (permalink / raw)
To: Arvind Sankar
Cc: linux-efi, the arch/x86 maintainers, Linux Kernel Mailing List
On Mon, 2 Mar 2020 at 17:54, Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> On Mon, Mar 02, 2020 at 08:49:17AM +0100, Ard Biesheuvel wrote:
> > On Mon, 2 Mar 2020 at 00:04, Arvind Sankar <nivedita@alum.mit.edu> 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)
?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/5] efi/x86: Make efi32_pe_entry more readable
2020-03-02 16:57 ` Ard Biesheuvel
@ 2020-03-02 17:02 ` Arvind Sankar
2020-03-02 17:09 ` Ard Biesheuvel
0 siblings, 1 reply; 12+ messages in thread
From: Arvind Sankar @ 2020-03-02 17:02 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Arvind Sankar, linux-efi, the arch/x86 maintainers,
Linux Kernel Mailing List
On Mon, Mar 02, 2020 at 05:57:04PM +0100, Ard Biesheuvel wrote:
> On Mon, 2 Mar 2020 at 17:54, Arvind Sankar <nivedita@alum.mit.edu> wrote:
> >
> > On Mon, Mar 02, 2020 at 08:49:17AM +0100, Ard Biesheuvel wrote:
> > > On Mon, 2 Mar 2020 at 00:04, Arvind Sankar <nivedita@alum.mit.edu> 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?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/5] efi/x86: Make efi32_pe_entry more readable
2020-03-02 17:02 ` Arvind Sankar
@ 2020-03-02 17:09 ` Ard Biesheuvel
0 siblings, 0 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2020-03-02 17:09 UTC (permalink / raw)
To: Arvind Sankar
Cc: linux-efi, the arch/x86 maintainers, Linux Kernel Mailing List
On Mon, 2 Mar 2020 at 18:02, Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> On Mon, Mar 02, 2020 at 05:57:04PM +0100, Ard Biesheuvel wrote:
> > On Mon, 2 Mar 2020 at 17:54, Arvind Sankar <nivedita@alum.mit.edu> wrote:
> > >
> > > On Mon, Mar 02, 2020 at 08:49:17AM +0100, Ard Biesheuvel wrote:
> > > > On Mon, 2 Mar 2020 at 00:04, Arvind Sankar <nivedita@alum.mit.edu> 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.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-03-02 17:09 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-01 23:04 [PATCH 0/5] efi/x86 cleanups and one bugfix Arvind Sankar
2020-03-01 23:04 ` [PATCH 1/5] efi/x86: Annotate the LOADED_IMAGE_PROTOCOL_GUID with SYM_DATA Arvind Sankar
2020-03-01 23:04 ` [PATCH 2/5] efi/x86: Respect 32-bit ABI in efi32_pe_entry Arvind Sankar
2020-03-01 23:04 ` [PATCH 3/5] efi/x86: Make efi32_pe_entry more readable Arvind Sankar
2020-03-02 7:49 ` Ard Biesheuvel
2020-03-02 16:54 ` Arvind Sankar
2020-03-02 16:57 ` Ard Biesheuvel
2020-03-02 17:02 ` Arvind Sankar
2020-03-02 17:09 ` Ard Biesheuvel
2020-03-01 23:04 ` [PATCH 4/5] efi/x86: Avoid using code32_start Arvind Sankar
2020-03-01 23:04 ` [PATCH 5/5] x86/boot: Use unsigned comparison for addresses Arvind Sankar
2020-03-02 7:50 ` [PATCH 0/5] efi/x86 cleanups and one bugfix Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).