Re: [PATCH 3/5] x86/ELF: don't store function pointer in elf_core_save_regs()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 28/09/2020 13:06, Jan Beulich wrote:
> This keeps at least gcc 10 from generating a separate function instance
> in common/kexec.o alongside the inlining of the function in its sole
> caller. I also think putting the address of the actual code storing the
> registers is a better indication to consumers than that of an otherwise
> unreferenced function.

Hmm - that's unfortunate.

elf_core_save_regs is certainly a useful name to spot in a backtrace.

> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> --- a/xen/include/asm-x86/x86_64/elf.h
> +++ b/xen/include/asm-x86/x86_64/elf.h
> @@ -54,7 +54,7 @@ static inline void elf_core_save_regs(EL
>      asm volatile("movq %%rsi,%0" : "=m"(core_regs->rsi));
>      asm volatile("movq %%rdi,%0" : "=m"(core_regs->rdi));
>      /* orig_rax not filled in for now */
> -    core_regs->rip = (unsigned long)elf_core_save_regs;
> +    asm volatile("call 0f; 0: popq %0" : "=m" (core_regs->rip));

lea 0(%rip) will be faster to execute, and this is 64bit code specifically.

Either way, Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

>      core_regs->cs = read_sreg(cs);
>      asm volatile("pushfq; popq %0" :"=m"(core_regs->rflags));
>      asm volatile("movq %%rsp,%0" : "=m"(core_regs->rsp));
>