Re: [U-Boot] [PATCH v2] x86: Add 64-bit setjmp/longjmp implementation

Hi Ivan,

On Thu, Jun 7, 2018 at 2:28 AM, Ivan Gorinov ivan.gorinov@intel.com wrote:

Add setjmp/longjmp functions for x86_64. The FPU control word and MXCSR control bits are preserved across calls.

Signed-off-by: Ivan Gorinov ivan.gorinov@intel.com

arch/x86/cpu/x86_64/setjmp.S | 66 +++++++++++++++++++++++++++++++++++++++++++ arch/x86/cpu/x86_64/setjmp.c | 19 ------------- arch/x86/include/asm/setjmp.h | 19 +++++++++++++ 3 files changed, 85 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..ef61a4a --- /dev/null +++ b/arch/x86/cpu/x86_64/setjmp.S @@ -0,0 +1,66 @@ +/*

• • Copyright (C) 2018 Intel Corporation
• • SPDX-License-Identifier: GPL-2.0+

This line should be put at the first line of this file, otherwise it causes checkpatch to report warnings.

• • See arch/x86/include/asm/setjmp.h for jmp_buf format
• */

+#include <linux/linkage.h>

+.text +.align 8

+ENTRY(setjmp)

•   pop     %rcx
  

•   movq    %rcx, (%rdi)    /* Return address */
  

•   movq    %rsp, 8 (%rdi)
  

nits: can we eliminate the space between 8 and (%edi)? and others in this file too?

•   movq    %rbp, 16 (%rdi)
  

•   movq    %rbx, 24 (%rdi)
  

•   movq    %r12, 32 (%rdi)
  

•   movq    %r13, 40 (%rdi)
  

•   movq    %r14, 48 (%rdi)
  

•   movq    %r15, 56 (%rdi)
  

•   fnstcw  64 (%rdi)
  

•   stmxcsr 68 (%rdi)
  

I don't think we need worry about these FP registers as U-Boot does not enable them at all. It looks your v1 patch does not include this but was added in v2. See the 32-bit setjmp/longjmp() implementation in U-Boot and there is no FP save/restore too.

•   xorq    %rax, %rax      /* Direct invocation returns 0 */
  

•   jmpq    *%rcx
  

+ENDPROC(setjmp)

+.align 8

+ENTRY(longjmp)

•   subq    $8, %rsp
  

+/* Restore the control bits of MXCSR */

nits: comment indention should align to the assembly code

•   stmxcsr (%rsp)
  

•   movl    $0x3f, %eax
  

•   andl    %eax, (%rsp)
  

•   notl    %eax
  

•   andl    68 (%rdi), %eax
  

•   orl     %eax, (%rsp)
  

•   ldmxcsr (%rsp)
  

•   fldcw   64 (%rdi)
  

•   movq    (%rdi), %rcx    /* Return address */
  

•   movq    8 (%rdi), %rsp
  

•   movq    16 (%rdi), %rbp
  

•   movq    24 (%rdi), %rbx
  

•   movq    32 (%rdi), %r12
  

•   movq    40 (%rdi), %r13
  

•   movq    48 (%rdi), %r14
  

•   movq    56 (%rdi), %r15
  

•   movq    %rsi, %rax      /* Value to be returned by setjmp() */
  

•   testq   %rax, %rax      /* cannot be 0 in this case */
  

•   jnz     1f
  

•   incq    %rax            /* Return 1 instead */
  

+1:

•   jmpq    *%rcx
  

+ENDPROC(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..eae33fb 100644 --- a/arch/x86/include/asm/setjmp.h +++ b/arch/x86/include/asm/setjmp.h @@ -8,6 +8,23 @@ #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;
  

•   unsigned int __fcw;
  

•   unsigned int __mxcsr;
  

+};

+#else

struct jmp_buf_data { unsigned int __ebx; unsigned int __esp; @@ -17,6 +34,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);

--

Regards, Bin