Excerpts from Christophe Leroy's message of February 9, 2021 4:13 pm:

Le 09/02/2021 à 03:00, Nicholas Piggin a écrit :

quoted

Excerpts from Christophe Leroy's message of February 9, 2021 1:10 am:

quoted

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 <redacted>
---
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.

quoted

+
  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