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 <[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.
>
> 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
_______________________________________________
U-Boot mailing list
[email protected]
https://lists.denx.de/listinfo/u-boot
