On 02.06.18 20:44, Heinrich Schuchardt wrote: > On 05/31/2018 12:50 AM, Ivan Gorinov wrote: >> Add setjmp/longjmp functions for x86_64, >> based on existing 32-bit implementation. > > Thank you for your patch. This addresses a gap that really should be closed. > > Please, add a line to the commit message indicating the calling > conventions that are supported. This is important because we use > different calling conventions in different places. > > One function where we need setjmp() is efi_start_image(). The function > is defined as EFIAPI. So we need an implementation supporting the ms_abi > calling convention.
I don't quite follow. The calling convention is declared in the C header (omitted means sysv, EFIAPI means ms). The compiler will adapt the call to the respective convention automatically. So if you call a sysv function from an EFIAPI function, gcc will push all registers that are potentially getting clobbered on the stack. So IIUC we really only need the sysv variant - which this patch implements, right? This also shouldn't need any mentioning, as it's the default throughout the code base, unless the function is annotated with the EFIAPI tag. Alex > > Another function where we use setjmp() is do_bootefi_exec() but this > function is defined as sysv_abi. > > I guess the patch relates to the sysv_abi calling convention: > > "System V Application Binary Interface: AMD64 Architecture Processor > Supplement (With LP64 and ILP32 Programming Models)" > https://software.intel.com/sites/default/files/article/402129/mpx-linux64-abi.pdf > > This document contains the following sentence: > > The control bits of the MXCSR register are callee-saved (preserved > across calls), while the status bits are caller-saved (not preserved). > I cannot see that this is reflected in the patch. > > See also https://msdn.microsoft.com/en-us/library/36d3b75w.aspx > > Your setjmp implemantion is not usable for the ms_abi that we need in > for efi_start_image(). Here you would have to save registers RBX, RBP, > RDI, RSI, RSP, R12, R13, R14, and R15. > > As reference have a look at this implementation: > https://github.com/freebsd/freebsd/blob/master/lib/libc/amd64/gen/setjmp.S > > Best regards > > Heinrich > >> >> Signed-off-by: Ivan Gorinov <[email protected]> > > > >> --- >> arch/x86/cpu/x86_64/setjmp.S | 56 >> +++++++++++++++++++++++++++++++++++++++++++ >> arch/x86/cpu/x86_64/setjmp.c | 19 --------------- >> arch/x86/include/asm/setjmp.h | 17 +++++++++++++ >> 3 files changed, 73 insertions(+), 19 deletions(-) >> create mode 100644 arch/x86/cpu/x86_64/setjmp.S >> delete mode 100644 arch/x86/cpu/x86_64/setjmp.c >> >> diff --git a/arch/x86/cpu/x86_64/setjmp.S b/arch/x86/cpu/x86_64/setjmp.S >> new file mode 100644 >> index 0000000..e10ee49 >> --- /dev/null >> +++ b/arch/x86/cpu/x86_64/setjmp.S >> @@ -0,0 +1,56 @@ >> +/* >> + * Based on arch/x86/cpu/i386/setjmp.S by H. Peter Anvin <[email protected]> >> + * >> + * SPDX-License-Identifier: GPL-2.0 >> + */ >> + >> +/* >> + * The jmp_buf is assumed to contain the following, in order: >> + * <return address> >> + * %rsp >> + * %rbp >> + * %rbx >> + * %r12 >> + * %r13 >> + * %r14 >> + * %r15 >> + */ >> + >> +.text >> +.align 8 >> +.globl setjmp >> +.type setjmp, @function >> + >> +setjmp: We have macros for asm provided functions (see arch/arm/lib/setjmp.S) to ensure that linker based section elimination works. Please make sure you use those. Thanks, Alex >> + pop %rcx >> + movq %rcx, (%rdi) /* Return address */ >> + movq %rsp, 8 (%rdi) /* Post-return %rsp! */ >> + movq %rbp, 16 (%rdi) >> + movq %rbx, 24 (%rdi) >> + movq %r12, 32 (%rdi) >> + movq %r13, 40 (%rdi) >> + movq %r14, 48 (%rdi) >> + movq %r15, 56 (%rdi) >> + xorq %rax, %rax /* Return value */ >> + jmpq *%rcx >> + >> +/* Provide function size if needed */ >> +.size setjmp, .-setjmp >> + >> +.align 8 >> +.globl longjmp >> +.type longjmp, @function >> + >> +longjmp: >> + movq 32 (%rdi), %r12 >> + movq 40 (%rdi), %r13 >> + movq 48 (%rdi), %r14 >> + movq 56 (%rdi), %r15 >> + movq 16 (%rdi), %rbp >> + movq 24 (%rdi), %rbx >> + movq 8 (%rdi), %rsp >> + movq (%rdi), %rcx >> + movq %rsi, %rdi >> + jmpq *%rcx >> + >> + .size longjmp, .-longjmp >> diff --git a/arch/x86/cpu/x86_64/setjmp.c b/arch/x86/cpu/x86_64/setjmp.c >> deleted file mode 100644 >> index 5d4a74a..0000000 >> --- a/arch/x86/cpu/x86_64/setjmp.c >> +++ /dev/null >> @@ -1,19 +0,0 @@ >> -// SPDX-License-Identifier: GPL-2.0+ >> -/* >> - * Copyright (c) 2016 Google, Inc >> - */ >> - >> -#include <common.h> >> -#include <asm/setjmp.h> >> - >> -int setjmp(struct jmp_buf_data *jmp_buf) >> -{ >> - printf("WARNING: setjmp() is not supported\n"); >> - >> - return 0; >> -} >> - >> -void longjmp(struct jmp_buf_data *jmp_buf, int val) >> -{ >> - printf("WARNING: longjmp() is not supported\n"); >> -} >> diff --git a/arch/x86/include/asm/setjmp.h b/arch/x86/include/asm/setjmp.h >> index f25975f..49c36c1 100644 >> --- a/arch/x86/include/asm/setjmp.h >> +++ b/arch/x86/include/asm/setjmp.h >> @@ -8,6 +8,21 @@ >> #ifndef __setjmp_h >> #define __setjmp_h >> >> +#ifdef CONFIG_X86_64 >> + >> +struct jmp_buf_data { >> + unsigned long __rip; >> + unsigned long __rsp; >> + unsigned long __rbp; >> + unsigned long __rbx; >> + unsigned long __r12; >> + unsigned long __r13; >> + unsigned long __r14; >> + unsigned long __r15; >> +}; >> + >> +#else >> + >> struct jmp_buf_data { >> unsigned int __ebx; >> unsigned int __esp; >> @@ -17,6 +32,8 @@ struct jmp_buf_data { >> unsigned int __eip; >> }; >> >> +#endif >> + >> int setjmp(struct jmp_buf_data *jmp_buf); >> void longjmp(struct jmp_buf_data *jmp_buf, int val); >> >> > _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
