| Message ID | 1470525796-26941-1-git-send-email-sjg@chromium.org |
|---|---|
| State | Superseded |
| Delegated to: | Bin Meng |
| Headers | show |
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. > + .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 /* */ > + 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? > + > + .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()? > + > +#endif > -- Regards, Bin
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
Hi Simon, On Wed, Aug 10, 2016 at 2:16 AM, Simon Glass <sjg@chromium.org> wrote: > 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. This does not look good to me. See efi_start_image(), there is a test against return value of setjmp(), but longjmp() does not provide a return value, which means the return value is undetermined. if (setjmp(&info->exit_jmp)) { /* We returned from the child image */ return EFI_EXIT(info->exit_status); } Regards, Bin
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) + */ + .text + .align 4 + .globl setjmp + .type setjmp, @function +setjmp: +#ifdef _REGPARM + movl %eax,%edx +#else + movl 4(%esp),%edx +#endif + popl %ecx # Return address, and adjust the stack + 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 + + .text + .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); + +#endif
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