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

[Nicholas Piggin <npiggin@gmail.com>]

Michal Suchánek's on August 29, 2019 1:30 am:
> On Tue, 27 Aug 2019 23:55:48 +1000
> Nicholas Piggin <npiggin@gmail.com> 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 <npiggin@gmail.com>
>> ---
>> 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);

Thanks I didn't see that.

> uapi/asm/ptrace.h defines user_pt_regs and asm/ptrace.h pt_regs.
> 
> I am not really sure which you wanted.

pt_regs. That is the struct that gets saved on the kernel stack by 
interrupts. user_pt_regs is part of the ptrace user ABI.

Thanks,
Nick