Re: [U-Boot] [PATCH 1/6] x86: Add implementations of setjmp() and longjmp()

Hi Bin,

On 9 August 2016 at 00:49, Bin Meng bmeng.cn@gmail.com wrote:

Hi Simon,

On Sun, Aug 7, 2016 at 7:23 AM, Simon Glass sjg@chromium.org wrote:

...

Bring in these functions from Linux v4.4. They will be needed for EFI loader support.

Signed-off-by: Simon Glass sjg@chromium.org

arch/x86/cpu/Makefile | 2 +- arch/x86/cpu/setjmp.S | 71 +++++++++++++++++++++++++++++++++++++++++++ arch/x86/include/asm/setjmp.h | 24 +++++++++++++++ 3 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 arch/x86/cpu/setjmp.S create mode 100644 arch/x86/include/asm/setjmp.h

diff --git a/arch/x86/cpu/Makefile b/arch/x86/cpu/Makefile index 2667e0b..f5b8c9e 100644 --- a/arch/x86/cpu/Makefile +++ b/arch/x86/cpu/Makefile @@ -10,7 +10,7 @@

extra-y = start.o obj-$(CONFIG_X86_RESET_VECTOR) += resetvec.o start16.o -obj-y += interrupts.o cpu.o cpu_x86.o call64.o +obj-y += interrupts.o cpu.o cpu_x86.o call64.o setjmp.o

AFLAGS_REMOVE_call32.o := -mregparm=3 \ $(if $(CONFIG_EFI_STUB_64BIT),-march=i386 -m32) diff --git a/arch/x86/cpu/setjmp.S b/arch/x86/cpu/setjmp.S new file mode 100644 index 0000000..24e7d8a --- /dev/null +++ b/arch/x86/cpu/setjmp.S @@ -0,0 +1,71 @@ +/*

• • Written by H. Peter Anvin hpa@zytor.com
• • Brought in from Linux v4.4 and modified for U-Boot
• • From Linux arch/um/sys-i386/setjmp.S
• • SPDX-License-Identifier: GPL-2.0
• */

+#include <asm/global_data.h> +#include <asm/msr-index.h> +#include <asm/processor-flags.h>

+#define _REGPARM

+/*

• • The jmp_buf is assumed to contain the following, in order:

• • %ebx
    

• • %esp
    

• • %ebp
    

• • %esi
    

• • %edi
    

• • <return address>
    

• */

•   /*
  

•    * rdi - 32-bit code segment selector
  

•    * rsi - target address
  

•    * rdx - table address (0 if none)
  

•    */
  

The comment above looks irrelevant.

OK, will drop.

...

•   .text
  

•   .align 4
  

•   .globl setjmp
  

•   .type setjmp, @function
  

+setjmp: +#ifdef _REGPARM

•   movl %eax,%edx
  

nits: needs space after ",". please fix this globally in this file.

...

+#else

•   movl 4(%esp),%edx
  

+#endif

•   popl %ecx                       # Return address, and adjust the stack
  

I believe # is deprecated. We should use /* */

OK

...

•   xorl %eax,%eax                  # Return value
  

•   movl %ebx,(%edx)
  

•   movl %esp,4(%edx)               # Post-return %esp!
  

•   pushl %ecx                      # Make the call/return stack happy
  

•   movl %ebp,8(%edx)
  

•   movl %esi,12(%edx)
  

•   movl %edi,16(%edx)
  

•   movl %ecx,20(%edx)              # Return address
  

•   ret
  

•   .size setjmp,.-setjmp
  

What is this .size for?

Size of the function code. It helps with nm --size-sort I think.

...

•   .text
  

nits: not necessary

...

•   .align 4
  

•   .globl longjmp
  

•   .type longjmp, @function
  

+longjmp: +#ifdef _REGPARM

•   xchgl %eax,%edx
  

+#else

•   movl 4(%esp),%edx               # jmp_ptr address
  

•   movl 8(%esp),%eax               # Return value
  

+#endif

•   movl (%edx),%ebx
  

•   movl 4(%edx),%esp
  

•   movl 8(%edx),%ebp
  

•   movl 12(%edx),%esi
  

•   movl 16(%edx),%edi
  

•   jmp *20(%edx)
  

•   .size longjmp,.-longjmp
  

diff --git a/arch/x86/include/asm/setjmp.h b/arch/x86/include/asm/setjmp.h new file mode 100644 index 0000000..deef67e --- /dev/null +++ b/arch/x86/include/asm/setjmp.h @@ -0,0 +1,24 @@ +/*

• • Written by H. Peter Anvin hpa@zytor.com
• • Brought in from Linux v4.4 and modified for U-Boot
• • From Linux arch/um/sys-i386/setjmp.S
• • SPDX-License-Identifier: GPL-2.0
• */

+#ifndef __setjmp_h +#define __setjmp_h

+struct jmp_buf_data {

•   unsigned int __ebx;
  

•   unsigned int __esp;
  

•   unsigned int __ebp;
  

•   unsigned int __esi;
  

•   unsigned int __edi;
  

•   unsigned int __eip;
  

+};

+int setjmp(struct jmp_buf_data *jmp_buf); +void longjmp(struct jmp_buf_data *jmp_buf);

shouldn't it be void longjmp(struct jmp_buf_data *jmp_buf, int val)? I am wondering how EFI loader could work with this longjmp()?

efi_exit() seems to not need the extra parameter.

...

+#endif

Regards, Bin

Regards, SImon