Excerpts from Christophe Leroy's message of February 9, 2021 4:13 pm: > > > Le 09/02/2021 à 03:00, Nicholas Piggin a écrit : >> Excerpts from Christophe Leroy's message of February 9, 2021 1:10 am: >>> Only PPC64 has scv. No need to check the 0x7ff0 trap on PPC32. >>> For that, add a helper trap_is_unsupported_scv() similar to >>> trap_is_scv(). >>> >>> And ignore the scv parameter in syscall_exit_prepare (Save 14 cycles >>> 346 => 332 cycles) >>> >>> Signed-off-by: Christophe Leroy <christophe.le...@csgroup.eu> >>> --- >>> v5: Added a helper trap_is_unsupported_scv() >>> --- >>> arch/powerpc/include/asm/ptrace.h | 5 +++++ >>> arch/powerpc/kernel/entry_32.S | 1 - >>> arch/powerpc/kernel/interrupt.c | 7 +++++-- >>> 3 files changed, 10 insertions(+), 3 deletions(-) >>> >>> diff --git a/arch/powerpc/include/asm/ptrace.h >>> b/arch/powerpc/include/asm/ptrace.h >>> index 58f9dc060a7b..2c842b11a924 100644 >>> --- a/arch/powerpc/include/asm/ptrace.h >>> +++ b/arch/powerpc/include/asm/ptrace.h >>> @@ -229,6 +229,11 @@ static inline bool trap_is_scv(struct pt_regs *regs) >>> return (IS_ENABLED(CONFIG_PPC_BOOK3S_64) && TRAP(regs) == 0x3000); >>> } >>> >>> +static inline bool trap_is_unsupported_scv(struct pt_regs *regs) >>> +{ >>> + return (IS_ENABLED(CONFIG_PPC_BOOK3S_64) && TRAP(regs) == 0x7ff0); >>> +} >> >> This change is good. >> >>> + >>> static inline bool trap_is_syscall(struct pt_regs *regs) >>> { >>> return (trap_is_scv(regs) || TRAP(regs) == 0xc00); >>> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S >>> index cffe58e63356..7c824e8928d0 100644 >>> --- a/arch/powerpc/kernel/entry_32.S >>> +++ b/arch/powerpc/kernel/entry_32.S >>> @@ -344,7 +344,6 @@ transfer_to_syscall: >>> >>> ret_from_syscall: >>> addi r4,r1,STACK_FRAME_OVERHEAD >>> - li r5,0 >>> bl syscall_exit_prepare >> >> For this one, I think it would be nice to do the "right" thing and make >> the function prototypes different on !64S. They could then declare a >> local const bool scv = 0. >> >> We could have syscall_exit_prepare and syscall_exit_prepare_maybe_scv >> or something like that, 64s can use the latter one and the former can be >> a wrapper that passes constant 0 for scv. Then we don't have different >> prototypes for the same function, but you just have to make the 32-bit >> version static inline and the 64-bit version exported to asm. > > You can't call a static inline function from ASM, I don't understand you.
I mean #ifdef CONFIG_PPC_BOOK3S_64 notrace unsigned long syscall_exit_prepare_scv(unsigned long r3, struct pt_regs *regs, long scv) #else static inline long syscall_exit_prepare_scv(unsigned long r3, struct pt_regs *regs, long scv) #endif #ifndef CONFIG_PPC_BOOK3S_64 notrace unsigned long syscall_exit_prepare(unsigned long r3, struct pt_regs *regs) { return syscall_exit_prepare_scv(r3, regs, 0); } #endif > > What is wrong for you really here ? Is that the fact we leave scv random, or > is that the below > IS_ENABLED() ? That scv arg is random. I know generated code essentially would be no different and no possibility of tracing, but would just prefer to call the C "correctly" if possible. > I don't mind keeping the 'li r5,0' before calling the function if you find it > cleaner, the real > performance gain is with setting scv to 0 below for PPC32 (and maybe it > should be set to zero for > book3e/64 too ?). Yes 64e would like this optimisation. Thanks, Nick