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

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

[Ivan Gorinov]

Add setjmp/longjmp functions for x86_64,
based on existing 32-bit implementation.

Signed-off-by: Ivan Gorinov <ivan.gorinov@intel.com>
---
 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 <hpa@zytor.com>
+ *
+ * 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:
+	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);
 
-- 
2.7.4

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

[Heinrich Schuchardt]

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.

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 <ivan.gorinov@intel.com>



> ---
>  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 <hpa@zytor.com>
> + *
> + * 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:
> +	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] [PATCH] x86: Add 64-bit setjmp/longjmp implementation

[Alexander Graf]

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 <ivan.gorinov@intel.com>
> 
> 
> 
>> ---
>>  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 <hpa@zytor.com>
>> + *
>> + * 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] [PATCH] x86: Add 64-bit setjmp/longjmp implementation

[Ivan Gorinov]

On Sun, Jun 03, 2018 at 02:06:47PM +0200, Alexander Graf wrote:
> 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.

I can confirm that gcc does save %rsi and %rdi in EFIAPI functions before
using those two registers for setjmp() parameters. The other callee-saved
registers in ms_abi are also callee-saved in sysv and correctly handled by
the proposed implementation.

static efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle,
					   unsigned long *exit_data_size,
					   s16 **exit_data)
{
    5f03ce56:	57                   	push   %rdi
    5f03ce57:	56                   	push   %rsi

    ...

	/* call the image! */
	if (setjmp(&info->exit_jmp)) {
    5f03cecd:	48 8b 84 24 f0 00 00 	mov    0xf0(%rsp),%rax
    5f03ced4:	00 
    5f03ced5:	48 8d 78 78          	lea    0x78(%rax),%rdi
    5f03ced9:	e8 82 32 fc ff       	callq  5f000160 <setjmp>
    5f03cede:	85 c0                	test   %eax,%eax
    5f03cee0:	74 1b                	je     5f03cefd <efi_start_image+0xa7>

    ...

> 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.

> > 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.

Thank you for the link! I will add MXCSR to the callee-saved register set.

> > 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
> > 
> >>
> >> Signed-off-by: Ivan Gorinov <ivan.gorinov@intel.com>
> > 
> > 
> > 
> >> ---
> >>  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 <hpa@zytor.com>
> >> + *
> >> + * 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.

OK. Shall I also add .pushsection/.popsection?
Can't find any example in arch/x86/

> >> +	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] [PATCH] x86: Add 64-bit setjmp/longjmp implementation

[Alexander Graf]

On 05.06.18 19:56, Ivan Gorinov wrote:
> On Sun, Jun 03, 2018 at 02:06:47PM +0200, Alexander Graf wrote:
>> 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.
> 
> I can confirm that gcc does save %rsi and %rdi in EFIAPI functions before
> using those two registers for setjmp() parameters. The other callee-saved
> registers in ms_abi are also callee-saved in sysv and correctly handled by
> the proposed implementation.
> 
> static efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle,
> 					   unsigned long *exit_data_size,
> 					   s16 **exit_data)
> {
>     5f03ce56:	57                   	push   %rdi
>     5f03ce57:	56                   	push   %rsi
> 
>     ...
> 
> 	/* call the image! */
> 	if (setjmp(&info->exit_jmp)) {
>     5f03cecd:	48 8b 84 24 f0 00 00 	mov    0xf0(%rsp),%rax
>     5f03ced4:	00 
>     5f03ced5:	48 8d 78 78          	lea    0x78(%rax),%rdi
>     5f03ced9:	e8 82 32 fc ff       	callq  5f000160 <setjmp>
>     5f03cede:	85 c0                	test   %eax,%eax
>     5f03cee0:	74 1b                	je     5f03cefd <efi_start_image+0xa7>
> 
>     ...
> 
>> 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.
> 
>>> 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.
> 
> Thank you for the link! I will add MXCSR to the callee-saved register set.
> 
>>> 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
>>>
>>>>
>>>> Signed-off-by: Ivan Gorinov <ivan.gorinov@intel.com>
>>>
>>>
>>>
>>>> ---
>>>>  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 <hpa@zytor.com>
>>>> + *
>>>> + * 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.
> 
> OK. Shall I also add .pushsection/.popsection?
> Can't find any example in arch/x86/

The ENTRY/ENDPROC macros are architecture agnostic, so you should just
be able to use them exactly like in the arm example above.


Alex