Subject: Re: [PATCH v2 4/4] powerpc/64: system call implement the bulk of the logic in C

From: Michal Suchánek <[email-masked]>
Date: Wed, 28 Aug 2019 17:30:49 +0200

On Tue, 27 Aug 2019 23:55:48 +1000
Nicholas Piggin <[email-masked]> wrote:

> System call entry and particularly exit code is beyond the limit of what
> is reasonable to implement in asm.
> 
> This conversion moves all conditional branches out of the asm code,
> except for the case that all GPRs should be restored at exit.
> 
> Null syscall test is about 5% faster after this patch, because the exit
> work is handled under local_irq_disable, and the hard mask and pending
> interrupt replay is handled after that, which avoids games with MSR.
> 
> Signed-off-by: Nicholas Piggin <[email-masked]>
> ---
> Changes since v1:
> - Improve changelog
> - Lot of code cleanups, moving helpers out to proper header locations,
>   etc (Christophe).
> - Split unnecessary change that affected ppc32 out. I will submit it
>   independently (Christophe).
> 
>  arch/powerpc/include/asm/asm-prototypes.h     |  11 -
>  .../powerpc/include/asm/book3s/64/kup-radix.h |  12 +-
>  arch/powerpc/include/asm/cputime.h            |  22 ++
>  arch/powerpc/include/asm/ptrace.h             |   3 +
>  arch/powerpc/include/asm/signal.h             |   2 +
>  arch/powerpc/include/asm/switch_to.h          |   5 +
>  arch/powerpc/include/asm/time.h               |   3 +
>  arch/powerpc/kernel/Makefile                  |   3 +-
>  arch/powerpc/kernel/entry_64.S                | 340 +++---------------
>  arch/powerpc/kernel/signal.h                  |   2 -
>  arch/powerpc/kernel/syscall_64.c              | 177 +++++++++
>  11 files changed, 273 insertions(+), 307 deletions(-)
>  create mode 100644 arch/powerpc/kernel/syscall_64.c
> 
> diff --git a/arch/powerpc/include/asm/asm-prototypes.h b/arch/powerpc/include/asm/asm-prototypes.h
> index ec1c97a8e8cb..f00ef8924a99 100644
> --- a/arch/powerpc/include/asm/asm-prototypes.h
> +++ b/arch/powerpc/include/asm/asm-prototypes.h
> @@ -92,14 +92,6 @@ long sys_switch_endian(void);
>  notrace unsigned int __check_irq_replay(void);
>  void notrace restore_interrupts(void);
>  
> -/* ptrace */
> -long do_syscall_trace_enter(struct pt_regs *regs);
> -void do_syscall_trace_leave(struct pt_regs *regs);
> -
> -/* process */
> -void restore_math(struct pt_regs *regs);
> -void restore_tm_state(struct pt_regs *regs);
> -
>  /* prom_init (OpenFirmware) */
>  unsigned long __init prom_init(unsigned long r3, unsigned long r4,
>  			       unsigned long pp,
> @@ -110,9 +102,6 @@ unsigned long __init prom_init(unsigned long r3, unsigned long r4,
>  void __init early_setup(unsigned long dt_ptr);
>  void early_setup_secondary(void);
>  
> -/* time */
> -void accumulate_stolen_time(void);
> -
>  /* misc runtime */
>  extern u64 __bswapdi2(u64);
>  extern s64 __lshrdi3(s64, int);
> diff --git a/arch/powerpc/include/asm/book3s/64/kup-radix.h b/arch/powerpc/include/asm/book3s/64/kup-radix.h
> index f254de956d6a..ef2e65ea8a73 100644
> --- a/arch/powerpc/include/asm/book3s/64/kup-radix.h
> +++ b/arch/powerpc/include/asm/book3s/64/kup-radix.h
> @@ -3,6 +3,7 @@
>  #define _ASM_POWERPC_BOOK3S_64_KUP_RADIX_H
>  
>  #include <linux/const.h>
> +#include <asm/reg.h>
>  
>  #define AMR_KUAP_BLOCK_READ	UL(0x4000000000000000)
>  #define AMR_KUAP_BLOCK_WRITE	UL(0x8000000000000000)
> @@ -56,7 +57,16 @@
>  
>  #ifdef CONFIG_PPC_KUAP
>  
> -#include <asm/reg.h>
> +#include <asm/mmu.h>
> +#include <asm/ptrace.h>
> +
> +static inline void kuap_check_amr(void)
> +{
> +#ifdef CONFIG_PPC_KUAP_DEBUG
> +	if (mmu_has_feature(MMU_FTR_RADIX_KUAP))
> +		WARN_ON_ONCE(mfspr(SPRN_AMR) != AMR_KUAP_BLOCKED);
> +#endif
> +}
>  
>  /*
>   * We support individually allowing read or write, but we don't support nesting
> diff --git a/arch/powerpc/include/asm/cputime.h b/arch/powerpc/include/asm/cputime.h
> index 2431b4ada2fa..f3aa9db1a3cc 100644
> --- a/arch/powerpc/include/asm/cputime.h
> +++ b/arch/powerpc/include/asm/cputime.h
> @@ -60,6 +60,28 @@ static inline void arch_vtime_task_switch(struct task_struct *prev)
>  }
>  #endif
>  
> +static inline void account_cpu_user_entry(void)
> +{
> +	unsigned long tb = mftb();
> +
> +	get_accounting(current)->utime += (tb - get_accounting(current)->starttime_user);
> +	get_accounting(current)->starttime = tb;
> +}
> +static inline void account_cpu_user_exit(void)
> +{
> +	unsigned long tb = mftb();
> +
> +	get_accounting(current)->stime += (tb - get_accounting(current)->starttime);
> +	get_accounting(current)->starttime_user = tb;
> +}
> +
>  #endif /* __KERNEL__ */
> +#else /* CONFIG_VIRT_CPU_ACCOUNTING_NATIVE */
> +static inline void account_cpu_user_entry(void)
> +{
> +}
> +static inline void account_cpu_user_exit(void)
> +{
> +}
>  #endif /* CONFIG_VIRT_CPU_ACCOUNTING_NATIVE */
>  #endif /* __POWERPC_CPUTIME_H */
> diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
> index feee1b21bbd5..af363086403a 100644
> --- a/arch/powerpc/include/asm/ptrace.h
> +++ b/arch/powerpc/include/asm/ptrace.h
> @@ -138,6 +138,9 @@ extern unsigned long profile_pc(struct pt_regs *regs);
>  #define profile_pc(regs) instruction_pointer(regs)
>  #endif
>  
> +long do_syscall_trace_enter(struct pt_regs *regs);
> +void do_syscall_trace_leave(struct pt_regs *regs);
> +
>  #define kernel_stack_pointer(regs) ((regs)->gpr[1])
>  static inline int is_syscall_success(struct pt_regs *regs)
>  {
> diff --git a/arch/powerpc/include/asm/signal.h b/arch/powerpc/include/asm/signal.h
> index 0803ca8b9149..0113be8dcb59 100644
> --- a/arch/powerpc/include/asm/signal.h
> +++ b/arch/powerpc/include/asm/signal.h
> @@ -6,4 +6,6 @@
>  #include <uapi/asm/signal.h>
>  #include <uapi/asm/ptrace.h>
>  
> +void do_notify_resume(struct pt_regs *regs, unsigned long thread_info_flags);
> +
/srv/kernel/arch/powerpc/include/asm/signal.h:9:30: warning: ‘struct pt_regs’ declared inside parameter list will not be visible outside of this definition or declaration
 void do_notify_resume(struct pt_regs *regs, unsigned long thread_info_flags);

uapi/asm/ptrace.h defines user_pt_regs and asm/ptrace.h pt_regs.

I am not really sure which you wanted.

Thanks

Michal