lkml.org 
[lkml]   [2014]   [Jul]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 6/7] x86_64,entry: Treat regs->ax the same in fastpath and slowpath syscalls
From
On Tue, Jul 15, 2014 at 12:32 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> For slowpath syscalls, we initialize regs->ax to -ENOSYS and stick
> the syscall number into regs->orig_ax prior to any possible tracing
> and syscall execution. This is user-visible ABI used by ptrace
> syscall emulation and seccomp.
>
> For fastpath syscalls, there's no good reason not to do the same
> thing. It's even slightly simpler than what we're currently doing.
> It probably has no measureable performance impact. It should have
> no user-visible effect.
>
> The purpose of this patch is to prepare for seccomp-based syscall
> emulation in the fast path. This change is just subtle enough that
> I'm keeping it separate.
>
> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
> ---
> arch/x86/include/asm/calling.h | 6 +++++-
> arch/x86/kernel/entry_64.S | 11 +++--------
> 2 files changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/include/asm/calling.h b/arch/x86/include/asm/calling.h
> index cb4c73b..76659b6 100644
> --- a/arch/x86/include/asm/calling.h
> +++ b/arch/x86/include/asm/calling.h
> @@ -85,7 +85,7 @@ For 32-bit we have the following conventions - kernel is built with
> #define ARGOFFSET R11
> #define SWFRAME ORIG_RAX
>
> - .macro SAVE_ARGS addskip=0, save_rcx=1, save_r891011=1
> + .macro SAVE_ARGS addskip=0, save_rcx=1, save_r891011=1, rax_enosys=0
> subq $9*8+\addskip, %rsp
> CFI_ADJUST_CFA_OFFSET 9*8+\addskip
> movq_cfi rdi, 8*8
> @@ -96,7 +96,11 @@ For 32-bit we have the following conventions - kernel is built with
> movq_cfi rcx, 5*8
> .endif
>
> + .if \rax_enosys
> + movq $-ENOSYS, 4*8(%rsp)
> + .else
> movq_cfi rax, 4*8
> + .endif
>
> .if \save_r891011
> movq_cfi r8, 3*8
> diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
> index b25ca96..432c190 100644
> --- a/arch/x86/kernel/entry_64.S
> +++ b/arch/x86/kernel/entry_64.S
> @@ -405,8 +405,8 @@ GLOBAL(system_call_after_swapgs)
> * and short:
> */
> ENABLE_INTERRUPTS(CLBR_NONE)
> - SAVE_ARGS 8,0
> - movq %rax,ORIG_RAX-ARGOFFSET(%rsp)
> + SAVE_ARGS 8, 0, rax_enosys=1
> + movq_cfi rax,(ORIG_RAX-ARGOFFSET)

I think changing store rax to macro is unnecessary,
since it breaks common style of asm with the next line:
> movq %rcx,RIP-ARGOFFSET(%rsp)
Also it made the diff harder to grasp.

The change from the next patch 7/7:

> - ja int_ret_from_sys_call /* RAX(%rsp) set to -ENOSYS above */
> + ja int_ret_from_sys_call /* RAX(%rsp) is already set */

probably belongs in this 6/7 patch as well.

The rest look good to me. I think it's a big improvement in readability
comparing to v1.


\
 
 \ /
  Last update: 2014-07-19 18:41    [W:0.208 / U:0.556 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site