* Re: [PATCH v7 32/42] powerpc/64: context tracking move to interrupt wrappers
From: Christophe Leroy @ 2021-02-09 5:49 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Athira Rajeev
In-Reply-To: <20210130130852.2952424-33-npiggin@gmail.com>
Le 30/01/2021 à 14:08, Nicholas Piggin a écrit :
> This moves exception_enter/exit calls to wrapper functions for
> synchronous interrupts. More interrupt handlers are covered by
> this than previously.
Why did you enclose everything in #ifdef CONFIG_PPC64 ? As far as I understand, before this patch
exception_enter() and exception_exit() are called also on PPC32.
Christophe
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/interrupt.h | 9 ++++
> arch/powerpc/kernel/traps.c | 74 ++++++---------------------
> arch/powerpc/mm/book3s64/hash_utils.c | 3 --
> arch/powerpc/mm/fault.c | 9 +---
> 4 files changed, 27 insertions(+), 68 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
> index 488bdd5bd922..e65ce3e2b071 100644
> --- a/arch/powerpc/include/asm/interrupt.h
> +++ b/arch/powerpc/include/asm/interrupt.h
> @@ -7,10 +7,16 @@
> #include <asm/ftrace.h>
>
> struct interrupt_state {
> +#ifdef CONFIG_PPC64
> + enum ctx_state ctx_state;
> +#endif
> };
>
> static inline void interrupt_enter_prepare(struct pt_regs *regs, struct interrupt_state *state)
> {
> +#ifdef CONFIG_PPC64
> + state->ctx_state = exception_enter();
> +#endif
> }
>
> /*
> @@ -29,6 +35,9 @@ static inline void interrupt_enter_prepare(struct pt_regs *regs, struct interrup
> */
> static inline void interrupt_exit_prepare(struct pt_regs *regs, struct interrupt_state *state)
> {
> +#ifdef CONFIG_PPC64
> + exception_exit(state->ctx_state);
> +#endif
> }
>
> static inline void interrupt_async_enter_prepare(struct pt_regs *regs, struct interrupt_state *state)
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index da488e62fb5f..21fd14828827 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -1087,41 +1087,28 @@ DEFINE_INTERRUPT_HANDLER_ASYNC(handle_hmi_exception)
>
> DEFINE_INTERRUPT_HANDLER(unknown_exception)
> {
> - enum ctx_state prev_state = exception_enter();
> -
> printk("Bad trap at PC: %lx, SR: %lx, vector=%lx\n",
> regs->nip, regs->msr, regs->trap);
>
> _exception(SIGTRAP, regs, TRAP_UNK, 0);
> -
> - exception_exit(prev_state);
> }
>
> DEFINE_INTERRUPT_HANDLER_ASYNC(unknown_async_exception)
> {
> - enum ctx_state prev_state = exception_enter();
> -
> printk("Bad trap at PC: %lx, SR: %lx, vector=%lx\n",
> regs->nip, regs->msr, regs->trap);
>
> _exception(SIGTRAP, regs, TRAP_UNK, 0);
> -
> - exception_exit(prev_state);
> }
>
> DEFINE_INTERRUPT_HANDLER(instruction_breakpoint_exception)
> {
> - enum ctx_state prev_state = exception_enter();
> -
> if (notify_die(DIE_IABR_MATCH, "iabr_match", regs, 5,
> 5, SIGTRAP) == NOTIFY_STOP)
> - goto bail;
> + return;
> if (debugger_iabr_match(regs))
> - goto bail;
> + return;
> _exception(SIGTRAP, regs, TRAP_BRKPT, regs->nip);
> -
> -bail:
> - exception_exit(prev_state);
> }
>
> DEFINE_INTERRUPT_HANDLER(RunModeException)
> @@ -1131,8 +1118,6 @@ DEFINE_INTERRUPT_HANDLER(RunModeException)
>
> DEFINE_INTERRUPT_HANDLER(single_step_exception)
> {
> - enum ctx_state prev_state = exception_enter();
> -
> clear_single_step(regs);
> clear_br_trace(regs);
>
> @@ -1141,14 +1126,11 @@ DEFINE_INTERRUPT_HANDLER(single_step_exception)
>
> if (notify_die(DIE_SSTEP, "single_step", regs, 5,
> 5, SIGTRAP) == NOTIFY_STOP)
> - goto bail;
> + return;
> if (debugger_sstep(regs))
> - goto bail;
> + return;
>
> _exception(SIGTRAP, regs, TRAP_TRACE, regs->nip);
> -
> -bail:
> - exception_exit(prev_state);
> }
> NOKPROBE_SYMBOL(single_step_exception);
>
> @@ -1476,7 +1458,6 @@ static inline int emulate_math(struct pt_regs *regs) { return -1; }
>
> DEFINE_INTERRUPT_HANDLER(program_check_exception)
> {
> - enum ctx_state prev_state = exception_enter();
> unsigned int reason = get_reason(regs);
>
> /* We can now get here via a FP Unavailable exception if the core
> @@ -1485,22 +1466,22 @@ DEFINE_INTERRUPT_HANDLER(program_check_exception)
> if (reason & REASON_FP) {
> /* IEEE FP exception */
> parse_fpe(regs);
> - goto bail;
> + return;
> }
> if (reason & REASON_TRAP) {
> unsigned long bugaddr;
> /* Debugger is first in line to stop recursive faults in
> * rcu_lock, notify_die, or atomic_notifier_call_chain */
> if (debugger_bpt(regs))
> - goto bail;
> + return;
>
> if (kprobe_handler(regs))
> - goto bail;
> + return;
>
> /* trap exception */
> if (notify_die(DIE_BPT, "breakpoint", regs, 5, 5, SIGTRAP)
> == NOTIFY_STOP)
> - goto bail;
> + return;
>
> bugaddr = regs->nip;
> /*
> @@ -1512,10 +1493,10 @@ DEFINE_INTERRUPT_HANDLER(program_check_exception)
> if (!(regs->msr & MSR_PR) && /* not user-mode */
> report_bug(bugaddr, regs) == BUG_TRAP_TYPE_WARN) {
> regs->nip += 4;
> - goto bail;
> + return;
> }
> _exception(SIGTRAP, regs, TRAP_BRKPT, regs->nip);
> - goto bail;
> + return;
> }
> #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> if (reason & REASON_TM) {
> @@ -1536,7 +1517,7 @@ DEFINE_INTERRUPT_HANDLER(program_check_exception)
> */
> if (user_mode(regs)) {
> _exception(SIGILL, regs, ILL_ILLOPN, regs->nip);
> - goto bail;
> + return;
> } else {
> printk(KERN_EMERG "Unexpected TM Bad Thing exception "
> "at %lx (msr 0x%lx) tm_scratch=%llx\n",
> @@ -1567,7 +1548,7 @@ DEFINE_INTERRUPT_HANDLER(program_check_exception)
> * pattern to occurrences etc. -dgibson 31/Mar/2003
> */
> if (!emulate_math(regs))
> - goto bail;
> + return;
>
> /* Try to emulate it if we should. */
> if (reason & (REASON_ILLEGAL | REASON_PRIVILEGED)) {
> @@ -1575,10 +1556,10 @@ DEFINE_INTERRUPT_HANDLER(program_check_exception)
> case 0:
> regs->nip += 4;
> emulate_single_step(regs);
> - goto bail;
> + return;
> case -EFAULT:
> _exception(SIGSEGV, regs, SEGV_MAPERR, regs->nip);
> - goto bail;
> + return;
> }
> }
>
> @@ -1587,9 +1568,6 @@ DEFINE_INTERRUPT_HANDLER(program_check_exception)
> _exception(SIGILL, regs, ILL_PRVOPC, regs->nip);
> else
> _exception(SIGILL, regs, ILL_ILLOPC, regs->nip);
> -
> -bail:
> - exception_exit(prev_state);
> }
> NOKPROBE_SYMBOL(program_check_exception);
>
> @@ -1606,14 +1584,12 @@ NOKPROBE_SYMBOL(emulation_assist_interrupt);
>
> DEFINE_INTERRUPT_HANDLER(alignment_exception)
> {
> - enum ctx_state prev_state = exception_enter();
> int sig, code, fixed = 0;
> unsigned long reason;
>
> interrupt_cond_local_irq_enable(regs);
>
> reason = get_reason(regs);
> -
> if (reason & REASON_BOUNDARY) {
> sig = SIGBUS;
> code = BUS_ADRALN;
> @@ -1621,7 +1597,7 @@ DEFINE_INTERRUPT_HANDLER(alignment_exception)
> }
>
> if (tm_abort_check(regs, TM_CAUSE_ALIGNMENT | TM_CAUSE_PERSISTENT))
> - goto bail;
> + return;
>
> /* we don't implement logging of alignment exceptions */
> if (!(current->thread.align_ctl & PR_UNALIGN_SIGBUS))
> @@ -1631,7 +1607,7 @@ DEFINE_INTERRUPT_HANDLER(alignment_exception)
> /* skip over emulated instruction */
> regs->nip += inst_length(reason);
> emulate_single_step(regs);
> - goto bail;
> + return;
> }
>
> /* Operand address was bad */
> @@ -1647,9 +1623,6 @@ DEFINE_INTERRUPT_HANDLER(alignment_exception)
> _exception(sig, regs, code, regs->dar);
> else
> bad_page_fault(regs, sig);
> -
> -bail:
> - exception_exit(prev_state);
> }
>
> DEFINE_INTERRUPT_HANDLER(StackOverflow)
> @@ -1663,41 +1636,28 @@ DEFINE_INTERRUPT_HANDLER(StackOverflow)
>
> DEFINE_INTERRUPT_HANDLER(stack_overflow_exception)
> {
> - enum ctx_state prev_state = exception_enter();
> -
> die("Kernel stack overflow", regs, SIGSEGV);
> -
> - exception_exit(prev_state);
> }
>
> DEFINE_INTERRUPT_HANDLER(kernel_fp_unavailable_exception)
> {
> - enum ctx_state prev_state = exception_enter();
> -
> printk(KERN_EMERG "Unrecoverable FP Unavailable Exception "
> "%lx at %lx\n", regs->trap, regs->nip);
> die("Unrecoverable FP Unavailable Exception", regs, SIGABRT);
> -
> - exception_exit(prev_state);
> }
>
> DEFINE_INTERRUPT_HANDLER(altivec_unavailable_exception)
> {
> - enum ctx_state prev_state = exception_enter();
> -
> if (user_mode(regs)) {
> /* A user program has executed an altivec instruction,
> but this kernel doesn't support altivec. */
> _exception(SIGILL, regs, ILL_ILLOPC, regs->nip);
> - goto bail;
> + return;
> }
>
> printk(KERN_EMERG "Unrecoverable VMX/Altivec Unavailable Exception "
> "%lx at %lx\n", regs->trap, regs->nip);
> die("Unrecoverable VMX/Altivec Unavailable Exception", regs, SIGABRT);
> -
> -bail:
> - exception_exit(prev_state);
> }
>
> DEFINE_INTERRUPT_HANDLER(vsx_unavailable_exception)
> diff --git a/arch/powerpc/mm/book3s64/hash_utils.c b/arch/powerpc/mm/book3s64/hash_utils.c
> index d681dc5a7b1c..fb7c10524bcd 100644
> --- a/arch/powerpc/mm/book3s64/hash_utils.c
> +++ b/arch/powerpc/mm/book3s64/hash_utils.c
> @@ -1514,7 +1514,6 @@ EXPORT_SYMBOL_GPL(hash_page);
> DECLARE_INTERRUPT_HANDLER_RET(__do_hash_fault);
> DEFINE_INTERRUPT_HANDLER_RET(__do_hash_fault)
> {
> - enum ctx_state prev_state = exception_enter();
> unsigned long ea = regs->dar;
> unsigned long dsisr = regs->dsisr;
> unsigned long access = _PAGE_PRESENT | _PAGE_READ;
> @@ -1563,8 +1562,6 @@ DEFINE_INTERRUPT_HANDLER_RET(__do_hash_fault)
> err = 0;
> }
>
> - exception_exit(prev_state);
> -
> return err;
> }
>
> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
> index 9c4220efc20f..b26a7643fc6e 100644
> --- a/arch/powerpc/mm/fault.c
> +++ b/arch/powerpc/mm/fault.c
> @@ -564,14 +564,7 @@ NOKPROBE_SYMBOL(__do_page_fault);
>
> DEFINE_INTERRUPT_HANDLER_RET(do_page_fault)
> {
> - enum ctx_state prev_state = exception_enter();
> - long err;
> -
> - err = __do_page_fault(regs);
> -
> - exception_exit(prev_state);
> -
> - return err;
> + return __do_page_fault(regs);
> }
> NOKPROBE_SYMBOL(do_page_fault);
>
>
^ permalink raw reply
* Re: [PATCH v5 20/22] powerpc/syscall: Avoid storing 'current' in another pointer
From: Nicholas Piggin @ 2021-02-09 2:36 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
msuchanek, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <24804747098369ebcdac38970b8f7a1260bdd248.1612796617.git.christophe.leroy@csgroup.eu>
Excerpts from Christophe Leroy's message of February 9, 2021 1:10 am:
> By saving the pointer pointing to thread_info.flags, gcc copies r2
> in a non-volatile register.
>
> We know 'current' doesn't change, so avoid that intermediaite pointer.
>
> Reduces null_syscall benchmark by 2 cycles (322 => 320 cycles)
>
> On PPC64, gcc seems to know that 'current' is not changing, and it keeps
> it in a non volatile register to avoid multiple read of 'current' in paca.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
What if you did this?
---
arch/powerpc/include/asm/current.h | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/current.h b/arch/powerpc/include/asm/current.h
index bbfb94800415..59ab327972a5 100644
--- a/arch/powerpc/include/asm/current.h
+++ b/arch/powerpc/include/asm/current.h
@@ -23,16 +23,19 @@ static inline struct task_struct *get_current(void)
return task;
}
-#define current get_current()
#else
-/*
- * We keep `current' in r2 for speed.
- */
-register struct task_struct *current asm ("r2");
+static inline struct task_struct *get_current(void)
+{
+ register struct task_struct *task asm ("r2");
+
+ return task;
+}
#endif
+#define current get_current()
+
#endif /* __KERNEL__ */
#endif /* _ASM_POWERPC_CURRENT_H */
--
^ permalink raw reply related
* Re: [PATCH v5 19/22] powerpc/syscall: Optimise checks in beginning of system_call_exception()
From: Nicholas Piggin @ 2021-02-09 2:06 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
msuchanek, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <3e48bb439357c6f72ae4343bf93bd29f0980eeb1.1612796617.git.christophe.leroy@csgroup.eu>
Excerpts from Christophe Leroy's message of February 9, 2021 1:10 am:
> Combine all tests of regs->msr into a single logical one.
Okay by me unless we choose to do the config option and put these all
under it. I think I would prefer that because sometimes the registers
are in a state you can't easily see what the values in the expression
were. In this case it doesn't matter so much because they should be in
regs in the interrupt frame.
Thanks,
Nick
>
> Before the patch:
>
> 0: 81 6a 00 84 lwz r11,132(r10)
> 4: 90 6a 00 88 stw r3,136(r10)
> 8: 69 60 00 02 xori r0,r11,2
> c: 54 00 ff fe rlwinm r0,r0,31,31,31
> 10: 0f 00 00 00 twnei r0,0
> 14: 69 63 40 00 xori r3,r11,16384
> 18: 54 63 97 fe rlwinm r3,r3,18,31,31
> 1c: 0f 03 00 00 twnei r3,0
> 20: 69 6b 80 00 xori r11,r11,32768
> 24: 55 6b 8f fe rlwinm r11,r11,17,31,31
> 28: 0f 0b 00 00 twnei r11,0
>
> After the patch:
>
> 0: 81 6a 00 84 lwz r11,132(r10)
> 4: 90 6a 00 88 stw r3,136(r10)
> 8: 7d 6b 58 f8 not r11,r11
> c: 71 6b c0 02 andi. r11,r11,49154
> 10: 0f 0b 00 00 twnei r11,0
>
> 6 cycles less on powerpc 8xx (328 => 322 cycles).
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/kernel/interrupt.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> index 55e1aa18cdb9..8c38e8c95be2 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -28,6 +28,7 @@ notrace long system_call_exception(long r3, long r4, long r5,
> unsigned long r0, struct pt_regs *regs)
> {
> syscall_fn f;
> + unsigned long expected_msr;
>
> regs->orig_gpr3 = r3;
>
> @@ -39,10 +40,13 @@ notrace long system_call_exception(long r3, long r4, long r5,
>
> trace_hardirqs_off(); /* finish reconciling */
>
> + expected_msr = MSR_PR;
> if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x))
> - BUG_ON(!(regs->msr & MSR_RI));
> - BUG_ON(!(regs->msr & MSR_PR));
> - BUG_ON(arch_irq_disabled_regs(regs));
> + expected_msr |= MSR_RI;
> + if (IS_ENABLED(CONFIG_PPC32))
> + expected_msr |= MSR_EE;
> + BUG_ON((regs->msr & expected_msr) ^ expected_msr);
> + BUG_ON(IS_ENABLED(CONFIG_PPC64) && arch_irq_disabled_regs(regs));
>
> #ifdef CONFIG_PPC_PKEY
> if (mmu_has_feature(MMU_FTR_PKEY)) {
> --
> 2.25.0
>
>
^ permalink raw reply
* Re: [PATCH v5 18/22] powerpc/syscall: Remove FULL_REGS verification in system_call_exception
From: Nicholas Piggin @ 2021-02-09 2:02 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
msuchanek, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <6bef4d9ba0cba50160d13e344ee4627ebdf801dc.1612796617.git.christophe.leroy@csgroup.eu>
Excerpts from Christophe Leroy's message of February 9, 2021 1:10 am:
> For book3s/64, FULL_REGS() is 'true' at all time, so the test voids.
> For others, non volatile registers are saved inconditionally.
>
> So the verification is pointless.
>
> Should one fail to do it, it would anyway be caught by the
> CHECK_FULL_REGS() in copy_thread() as we have removed the
> special versions ppc_fork() and friends.
>
> null_syscall benchmark reduction 4 cycles (332 => 328 cycles)
I wonder if we rather make a CONFIG option for a bunch of these simpler
debug checks here (and also in interrupt exit, wrappers, etc) rather
than remove them entirely.
Thanks,
Nick
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/kernel/interrupt.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> index 8fafca727b8b..55e1aa18cdb9 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -42,7 +42,6 @@ notrace long system_call_exception(long r3, long r4, long r5,
> if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x))
> BUG_ON(!(regs->msr & MSR_RI));
> BUG_ON(!(regs->msr & MSR_PR));
> - BUG_ON(!FULL_REGS(regs));
> BUG_ON(arch_irq_disabled_regs(regs));
>
> #ifdef CONFIG_PPC_PKEY
> --
> 2.25.0
>
>
^ permalink raw reply
* Re: [PATCH v5 17/22] powerpc/syscall: Do not check unsupported scv vector on PPC32
From: Nicholas Piggin @ 2021-02-09 2:00 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
msuchanek, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <fc3afe1870f943b2010805fcb045b718a638b3c6.1612796617.git.christophe.leroy@csgroup.eu>
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.leroy@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.
Thanks,
Nick
> #if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
> /* If the process has its own DBCR0 value, load it up. The internal
> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> index 205902052112..8fafca727b8b 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -88,7 +88,7 @@ notrace long system_call_exception(long r3, long r4, long r5,
> local_irq_enable();
>
> if (unlikely(current_thread_info()->flags & _TIF_SYSCALL_DOTRACE)) {
> - if (unlikely(regs->trap == 0x7ff0)) {
> + if (unlikely(trap_is_unsupported_scv(regs))) {
> /* Unsupported scv vector */
> _exception(SIGILL, regs, ILL_ILLOPC, regs->nip);
> return regs->gpr[3];
> @@ -111,7 +111,7 @@ notrace long system_call_exception(long r3, long r4, long r5,
> r8 = regs->gpr[8];
>
> } else if (unlikely(r0 >= NR_syscalls)) {
> - if (unlikely(regs->trap == 0x7ff0)) {
> + if (unlikely(trap_is_unsupported_scv(regs))) {
> /* Unsupported scv vector */
> _exception(SIGILL, regs, ILL_ILLOPC, regs->nip);
> return regs->gpr[3];
> @@ -224,6 +224,9 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
> unsigned long ti_flags;
> unsigned long ret = 0;
>
> + if (IS_ENABLED(CONFIG_PPC32))
> + scv = 0;
> +
> CT_WARN_ON(ct_state() == CONTEXT_USER);
>
> #ifdef CONFIG_PPC64
> --
> 2.25.0
>
>
^ permalink raw reply
* Re: [PATCH v5 16/22] powerpc/syscall: Avoid stack frame in likely part of system_call_exception()
From: Nicholas Piggin @ 2021-02-09 1:55 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
msuchanek, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <981edfd50d4c980634b74c4bb76b765c499a87ec.1612796617.git.christophe.leroy@csgroup.eu>
Excerpts from Christophe Leroy's message of February 9, 2021 1:10 am:
> When r3 is not modified, reload it from regs->orig_r3 to free
> volatile registers. This avoids a stack frame for the likely part
> of system_call_exception()
This doesn't on my 64s build, but it does reduce one non volatile
register save/restore. With quite a bit more register pressure
reduction 64s can avoid the stack frame as well.
It's a cool trick but quite code and compiler specific so I don't know
how worthwhile it is to keep considering we're calling out into random
kernel C code after this.
Maybe just keep it PPC32 specific for the moment, will have to do more
tuning for 64 and we have other stuff to do there first.
If you are happy to make it 32-bit only then
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
>
> Before the patch:
>
> c000b4d4 <system_call_exception>:
> c000b4d4: 7c 08 02 a6 mflr r0
> c000b4d8: 94 21 ff e0 stwu r1,-32(r1)
> c000b4dc: 93 e1 00 1c stw r31,28(r1)
> c000b4e0: 90 01 00 24 stw r0,36(r1)
> c000b4e4: 90 6a 00 88 stw r3,136(r10)
> c000b4e8: 81 6a 00 84 lwz r11,132(r10)
> c000b4ec: 69 6b 00 02 xori r11,r11,2
> c000b4f0: 55 6b ff fe rlwinm r11,r11,31,31,31
> c000b4f4: 0f 0b 00 00 twnei r11,0
> c000b4f8: 81 6a 00 a0 lwz r11,160(r10)
> c000b4fc: 55 6b 07 fe clrlwi r11,r11,31
> c000b500: 0f 0b 00 00 twnei r11,0
> c000b504: 7c 0c 42 e6 mftb r0
> c000b508: 83 e2 00 08 lwz r31,8(r2)
> c000b50c: 81 82 00 28 lwz r12,40(r2)
> c000b510: 90 02 00 24 stw r0,36(r2)
> c000b514: 7d 8c f8 50 subf r12,r12,r31
> c000b518: 7c 0c 02 14 add r0,r12,r0
> c000b51c: 90 02 00 08 stw r0,8(r2)
> c000b520: 7c 10 13 a6 mtspr 80,r0
> c000b524: 81 62 00 70 lwz r11,112(r2)
> c000b528: 71 60 86 91 andi. r0,r11,34449
> c000b52c: 40 82 00 34 bne c000b560 <system_call_exception+0x8c>
> c000b530: 2b 89 01 b6 cmplwi cr7,r9,438
> c000b534: 41 9d 00 64 bgt cr7,c000b598 <system_call_exception+0xc4>
> c000b538: 3d 40 c0 5c lis r10,-16292
> c000b53c: 55 29 10 3a rlwinm r9,r9,2,0,29
> c000b540: 39 4a 41 e8 addi r10,r10,16872
> c000b544: 80 01 00 24 lwz r0,36(r1)
> c000b548: 7d 2a 48 2e lwzx r9,r10,r9
> c000b54c: 7c 08 03 a6 mtlr r0
> c000b550: 7d 29 03 a6 mtctr r9
> c000b554: 83 e1 00 1c lwz r31,28(r1)
> c000b558: 38 21 00 20 addi r1,r1,32
> c000b55c: 4e 80 04 20 bctr
>
> After the patch:
>
> c000b4d4 <system_call_exception>:
> c000b4d4: 81 6a 00 84 lwz r11,132(r10)
> c000b4d8: 90 6a 00 88 stw r3,136(r10)
> c000b4dc: 69 6b 00 02 xori r11,r11,2
> c000b4e0: 55 6b ff fe rlwinm r11,r11,31,31,31
> c000b4e4: 0f 0b 00 00 twnei r11,0
> c000b4e8: 80 6a 00 a0 lwz r3,160(r10)
> c000b4ec: 54 63 07 fe clrlwi r3,r3,31
> c000b4f0: 0f 03 00 00 twnei r3,0
> c000b4f4: 7d 6c 42 e6 mftb r11
> c000b4f8: 81 82 00 08 lwz r12,8(r2)
> c000b4fc: 80 02 00 28 lwz r0,40(r2)
> c000b500: 91 62 00 24 stw r11,36(r2)
> c000b504: 7c 00 60 50 subf r0,r0,r12
> c000b508: 7d 60 5a 14 add r11,r0,r11
> c000b50c: 91 62 00 08 stw r11,8(r2)
> c000b510: 7c 10 13 a6 mtspr 80,r0
> c000b514: 80 62 00 70 lwz r3,112(r2)
> c000b518: 70 6b 86 91 andi. r11,r3,34449
> c000b51c: 40 82 00 28 bne c000b544 <system_call_exception+0x70>
> c000b520: 2b 89 01 b6 cmplwi cr7,r9,438
> c000b524: 41 9d 00 84 bgt cr7,c000b5a8 <system_call_exception+0xd4>
> c000b528: 80 6a 00 88 lwz r3,136(r10)
> c000b52c: 3d 40 c0 5c lis r10,-16292
> c000b530: 55 29 10 3a rlwinm r9,r9,2,0,29
> c000b534: 39 4a 41 e4 addi r10,r10,16868
> c000b538: 7d 2a 48 2e lwzx r9,r10,r9
> c000b53c: 7d 29 03 a6 mtctr r9
> c000b540: 4e 80 04 20 bctr
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/kernel/interrupt.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> index 107ec39f05cb..205902052112 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -117,6 +117,9 @@ notrace long system_call_exception(long r3, long r4, long r5,
> return regs->gpr[3];
> }
> return -ENOSYS;
> + } else {
> + /* Restore r3 from orig_gpr3 to free up a volatile reg */
> + r3 = regs->orig_gpr3;
> }
>
> /* May be faster to do array_index_nospec? */
> --
> 2.25.0
>
>
^ permalink raw reply
* Re: [PATCH v5 12/22] powerpc/syscall: Change condition to check MSR_RI
From: Nicholas Piggin @ 2021-02-09 1:36 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
msuchanek, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <67820fada8dd6a8fe9d7b666f175d4cc9d8de87e.1612796617.git.christophe.leroy@csgroup.eu>
Excerpts from Christophe Leroy's message of February 9, 2021 1:10 am:
> In system_call_exception(), MSR_RI also needs to be checked on 8xx.
> Only booke and 40x doesn't have MSR_RI.
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
...
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> v5: Also in interrupt exit prepare
> ---
> arch/powerpc/kernel/interrupt.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> index 1a2dec49f811..107ec39f05cb 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -39,7 +39,7 @@ notrace long system_call_exception(long r3, long r4, long r5,
>
> trace_hardirqs_off(); /* finish reconciling */
>
> - if (IS_ENABLED(CONFIG_PPC_BOOK3S))
> + if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x))
> BUG_ON(!(regs->msr & MSR_RI));
> BUG_ON(!(regs->msr & MSR_PR));
> BUG_ON(!FULL_REGS(regs));
> @@ -338,7 +338,7 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned
> unsigned long flags;
> unsigned long ret = 0;
>
> - if (IS_ENABLED(CONFIG_PPC_BOOK3S))
> + if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x))
> BUG_ON(!(regs->msr & MSR_RI));
> BUG_ON(!(regs->msr & MSR_PR));
> BUG_ON(!FULL_REGS(regs));
> @@ -436,7 +436,8 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign
> unsigned long amr;
> #endif
>
> - if (IS_ENABLED(CONFIG_PPC_BOOK3S) && unlikely(!(regs->msr & MSR_RI)))
> + if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x) &&
> + unlikely(!(regs->msr & MSR_RI)))
> unrecoverable_exception(regs);
> BUG_ON(regs->msr & MSR_PR);
> BUG_ON(!FULL_REGS(regs));
> --
> 2.25.0
>
>
^ permalink raw reply
* Re: [PATCH v5 11/22] powerpc/syscall: Save r3 in regs->orig_r3
From: Nicholas Piggin @ 2021-02-09 1:29 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
msuchanek, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <9a90805ab6b9101b46daf56470f457a57acd86fc.1612796617.git.christophe.leroy@csgroup.eu>
Excerpts from Christophe Leroy's message of February 9, 2021 1:10 am:
> Save r3 in regs->orig_r3 in system_call_exception()
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> v5: Removed the assembly one on SCV type system call
> ---
> arch/powerpc/kernel/entry_64.S | 2 --
> arch/powerpc/kernel/interrupt.c | 2 ++
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> index 33ddfeef4fe9..a91c2def165d 100644
> --- a/arch/powerpc/kernel/entry_64.S
> +++ b/arch/powerpc/kernel/entry_64.S
> @@ -108,7 +108,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_TM)
> li r11,\trapnr
> std r11,_TRAP(r1)
> std r12,_CCR(r1)
> - std r3,ORIG_GPR3(r1)
> addi r10,r1,STACK_FRAME_OVERHEAD
> ld r11,exception_marker@toc(r2)
> std r11,-16(r10) /* "regshere" marker */
> @@ -278,7 +277,6 @@ END_BTB_FLUSH_SECTION
> std r10,_LINK(r1)
> std r11,_TRAP(r1)
> std r12,_CCR(r1)
> - std r3,ORIG_GPR3(r1)
> addi r10,r1,STACK_FRAME_OVERHEAD
> ld r11,exception_marker@toc(r2)
> std r11,-16(r10) /* "regshere" marker */
> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> index 46fd195ca659..1a2dec49f811 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -29,6 +29,8 @@ notrace long system_call_exception(long r3, long r4, long r5,
> {
> syscall_fn f;
>
> + regs->orig_gpr3 = r3;
> +
> if (IS_ENABLED(CONFIG_PPC_IRQ_SOFT_MASK_DEBUG))
> BUG_ON(irq_soft_mask_return() != IRQS_ALL_DISABLED);
>
> --
> 2.25.0
>
>
^ permalink raw reply
* Re: [PATCH v5 10/22] powerpc/syscall: Use is_compat_task()
From: Nicholas Piggin @ 2021-02-09 1:29 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
msuchanek, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <c8094662199337a7200fea9f6e1d1f8b1b6d5f69.1612796617.git.christophe.leroy@csgroup.eu>
Excerpts from Christophe Leroy's message of February 9, 2021 1:10 am:
> Instead of hard comparing task flags with _TIF_32BIT, use
> is_compat_task(). The advantage is that it returns 0 on PPC32
> allthough _TIF_32BIT is always set.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/kernel/interrupt.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> index 2dac4d2bb1cf..46fd195ca659 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -2,6 +2,8 @@
>
> #include <linux/context_tracking.h>
> #include <linux/err.h>
> +#include <linux/compat.h>
> +
> #include <asm/asm-prototypes.h>
> #include <asm/kup.h>
> #include <asm/cputime.h>
> @@ -118,7 +120,7 @@ notrace long system_call_exception(long r3, long r4, long r5,
> /* May be faster to do array_index_nospec? */
> barrier_nospec();
>
> - if (unlikely(is_32bit_task())) {
> + if (unlikely(is_compat_task())) {
> f = (void *)compat_sys_call_table[r0];
>
> r3 &= 0x00000000ffffffffULL;
> --
> 2.25.0
>
>
^ permalink raw reply
* Re: [PATCH v5 09/22] powerpc/syscall: Make interrupt.c buildable on PPC32
From: Nicholas Piggin @ 2021-02-09 1:27 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
msuchanek, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <ba073ad67bd971a88ce331b65d6655523b54c794.1612796617.git.christophe.leroy@csgroup.eu>
Excerpts from Christophe Leroy's message of February 9, 2021 1:10 am:
> To allow building interrupt.c on PPC32, ifdef out specific PPC64
> code or use helpers which are available on both PP32 and PPC64
>
> Modify Makefile to always build interrupt.o
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> v5:
> - Also for interrupt exit preparation
> - Opted out kuap related code, ppc32 keeps it in ASM for the time being
> ---
> arch/powerpc/kernel/Makefile | 4 ++--
> arch/powerpc/kernel/interrupt.c | 31 ++++++++++++++++++++++++-------
> 2 files changed, 26 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index 26ff8c6e06b7..163755b1cef4 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -57,10 +57,10 @@ obj-y := cputable.o syscalls.o \
> prom.o traps.o setup-common.o \
> udbg.o misc.o io.o misc_$(BITS).o \
> of_platform.o prom_parse.o firmware.o \
> - hw_breakpoint_constraints.o
> + hw_breakpoint_constraints.o interrupt.o
> obj-y += ptrace/
> obj-$(CONFIG_PPC64) += setup_64.o \
> - paca.o nvram_64.o note.o interrupt.o
> + paca.o nvram_64.o note.o
> obj-$(CONFIG_COMPAT) += sys_ppc32.o signal_32.o
> obj-$(CONFIG_VDSO32) += vdso32_wrapper.o
> obj-$(CONFIG_PPC_WATCHDOG) += watchdog.o
> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> index d6be4f9a67e5..2dac4d2bb1cf 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -39,7 +39,7 @@ notrace long system_call_exception(long r3, long r4, long r5,
> BUG_ON(!(regs->msr & MSR_RI));
> BUG_ON(!(regs->msr & MSR_PR));
> BUG_ON(!FULL_REGS(regs));
> - BUG_ON(regs->softe != IRQS_ENABLED);
> + BUG_ON(arch_irq_disabled_regs(regs));
>
> #ifdef CONFIG_PPC_PKEY
> if (mmu_has_feature(MMU_FTR_PKEY)) {
> @@ -65,7 +65,9 @@ notrace long system_call_exception(long r3, long r4, long r5,
> isync();
> } else
> #endif
> +#ifdef CONFIG_PPC64
> kuap_check_amr();
> +#endif
Wouldn't mind trying to get rid of these ifdefs at some point, but
there's some kuap / keys changes going on recently so I'm happy enough
to let this settle then look at whether we can refactor.
>
> account_cpu_user_entry();
>
> @@ -77,7 +79,7 @@ notrace long system_call_exception(long r3, long r4, long r5,
> * frame, or if the unwinder was taught the first stack frame always
> * returns to user with IRQS_ENABLED, this store could be avoided!
> */
> - regs->softe = IRQS_ENABLED;
> + irq_soft_mask_regs_set_state(regs, IRQS_ENABLED);
>
> local_irq_enable();
>
> @@ -151,6 +153,7 @@ static notrace inline bool __prep_irq_for_enabled_exit(bool clear_ri)
> __hard_EE_RI_disable();
> else
> __hard_irq_disable();
> +#ifdef CONFIG_PPC64
> if (unlikely(lazy_irq_pending_nocheck())) {
> /* Took an interrupt, may have more exit work to do. */
> if (clear_ri)
> @@ -162,7 +165,7 @@ static notrace inline bool __prep_irq_for_enabled_exit(bool clear_ri)
> }
> local_paca->irq_happened = 0;
> irq_soft_mask_set(IRQS_ENABLED);
> -
> +#endif
> return true;
> }
>
Do we prefer space before return except in trivial cases?
> @@ -216,7 +219,9 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
>
> CT_WARN_ON(ct_state() == CONTEXT_USER);
>
> +#ifdef CONFIG_PPC64
> kuap_check_amr();
> +#endif
>
> regs->result = r3;
>
> @@ -309,7 +314,7 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
>
> account_cpu_user_exit();
>
> -#ifdef CONFIG_PPC_BOOK3S /* BOOK3E not yet using this */
> +#ifdef CONFIG_PPC_BOOK3S_64 /* BOOK3E and ppc32 not using this */
> /*
> * We do this at the end so that we do context switch with KERNEL AMR
> */
> @@ -318,7 +323,7 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
> return ret;
> }
>
> -#ifdef CONFIG_PPC_BOOK3S /* BOOK3E not yet using this */
> +#ifndef CONFIG_PPC_BOOK3E_64 /* BOOK3E not yet using this */
> notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned long msr)
> {
> #ifdef CONFIG_PPC_BOOK3E
Why are you building this for 32? I don't mind if it's just to keep
things similar and make it build for now, but you're not using it yet,
right?
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> @@ -333,14 +338,16 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned
> BUG_ON(!(regs->msr & MSR_RI));
> BUG_ON(!(regs->msr & MSR_PR));
> BUG_ON(!FULL_REGS(regs));
> - BUG_ON(regs->softe != IRQS_ENABLED);
> + BUG_ON(arch_irq_disabled_regs(regs));
> CT_WARN_ON(ct_state() == CONTEXT_USER);
>
> /*
> * We don't need to restore AMR on the way back to userspace for KUAP.
> * AMR can only have been unlocked if we interrupted the kernel.
> */
> +#ifdef CONFIG_PPC64
> kuap_check_amr();
> +#endif
>
> local_irq_save(flags);
>
> @@ -407,7 +414,9 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned
> /*
> * We do this at the end so that we do context switch with KERNEL AMR
> */
> +#ifdef CONFIG_PPC64
> kuap_user_restore(regs);
> +#endif
> return ret;
> }
>
> @@ -419,7 +428,9 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign
> unsigned long *ti_flagsp = ¤t_thread_info()->flags;
> unsigned long flags;
> unsigned long ret = 0;
> +#ifdef CONFIG_PPC64
> unsigned long amr;
> +#endif
>
> if (IS_ENABLED(CONFIG_PPC_BOOK3S) && unlikely(!(regs->msr & MSR_RI)))
> unrecoverable_exception(regs);
> @@ -432,7 +443,9 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign
> if (TRAP(regs) != 0x700)
> CT_WARN_ON(ct_state() == CONTEXT_USER);
>
> +#ifdef CONFIG_PPC64
> amr = kuap_get_and_check_amr();
> +#endif
>
> if (unlikely(*ti_flagsp & _TIF_EMULATE_STACK_STORE)) {
> clear_bits(_TIF_EMULATE_STACK_STORE, ti_flagsp);
> @@ -441,7 +454,7 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign
>
> local_irq_save(flags);
>
> - if (regs->softe == IRQS_ENABLED) {
> + if (!arch_irq_disabled_regs(regs)) {
> /* Returning to a kernel context with local irqs enabled. */
> WARN_ON_ONCE(!(regs->msr & MSR_EE));
> again:
> @@ -458,8 +471,10 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign
> } else {
> /* Returning to a kernel context with local irqs disabled. */
> __hard_EE_RI_disable();
> +#ifdef CONFIG_PPC64
> if (regs->msr & MSR_EE)
> local_paca->irq_happened &= ~PACA_IRQ_HARD_DIS;
> +#endif
> }
>
>
> @@ -472,7 +487,9 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign
> * which would cause Read-After-Write stalls. Hence, we take the AMR
> * value from the check above.
> */
> +#ifdef CONFIG_PPC64
> kuap_kernel_restore(regs, amr);
> +#endif
>
> return ret;
> }
> --
> 2.25.0
>
>
^ permalink raw reply
* Re: [PATCH v5 08/22] powerpc/syscall: Rename syscall_64.c into interrupt.c
From: Nicholas Piggin @ 2021-02-09 1:19 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
msuchanek, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cddc2deaa8f049d3ec419738e69804934919b935.1612796617.git.christophe.leroy@csgroup.eu>
Excerpts from Christophe Leroy's message of February 9, 2021 1:10 am:
> syscall_64.c will be reused almost as is for PPC32.
>
> As this file also contains functions to handle other types
> of interrupts rename it interrupt.c
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/kernel/Makefile | 2 +-
> arch/powerpc/kernel/{syscall_64.c => interrupt.c} | 0
> 2 files changed, 1 insertion(+), 1 deletion(-)
> rename arch/powerpc/kernel/{syscall_64.c => interrupt.c} (100%)
>
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index c173efd66c00..26ff8c6e06b7 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -60,7 +60,7 @@ obj-y := cputable.o syscalls.o \
> hw_breakpoint_constraints.o
> obj-y += ptrace/
> obj-$(CONFIG_PPC64) += setup_64.o \
> - paca.o nvram_64.o note.o syscall_64.o
> + paca.o nvram_64.o note.o interrupt.o
> obj-$(CONFIG_COMPAT) += sys_ppc32.o signal_32.o
> obj-$(CONFIG_VDSO32) += vdso32_wrapper.o
> obj-$(CONFIG_PPC_WATCHDOG) += watchdog.o
> diff --git a/arch/powerpc/kernel/syscall_64.c b/arch/powerpc/kernel/interrupt.c
> similarity index 100%
> rename from arch/powerpc/kernel/syscall_64.c
> rename to arch/powerpc/kernel/interrupt.c
> --
> 2.25.0
>
>
^ permalink raw reply
* Re: [PATCH v5 07/22] powerpc/irq: Add stub irq_soft_mask_return() for PPC32
From: Nicholas Piggin @ 2021-02-09 1:19 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
msuchanek, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <9b9f62c5e2e63cc121fd749a923aaaee92ee0da4.1612796617.git.christophe.leroy@csgroup.eu>
Excerpts from Christophe Leroy's message of February 9, 2021 1:10 am:
> To allow building syscall_64.c smoothly on PPC32, add stub version
> of irq_soft_mask_return().
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Same kind of comment as the other soft mask stuff. Again not a big deal
but there might be a way to improve it. For example make a
debug_syscall_entry(regs) function that ppc64 could put the soft mask
checks into.
No big deal, if you don't make any changes now I might see about doing
something like that after your series goes in.
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/hw_irq.h | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h
> index 4739f61e632c..56a98936a6a9 100644
> --- a/arch/powerpc/include/asm/hw_irq.h
> +++ b/arch/powerpc/include/asm/hw_irq.h
> @@ -330,6 +330,11 @@ static inline void irq_soft_mask_regs_set_state(struct pt_regs *regs, unsigned l
> }
> #else /* CONFIG_PPC64 */
>
> +static inline notrace unsigned long irq_soft_mask_return(void)
> +{
> + return 0;
> +}
> +
> static inline unsigned long arch_local_save_flags(void)
> {
> return mfmsr();
> --
> 2.25.0
>
>
^ permalink raw reply
* Re: [PATCH v5 06/22] powerpc/irq: Rework helpers that manipulate MSR[EE/RI]
From: Nicholas Piggin @ 2021-02-09 1:14 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
msuchanek, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <0e290372a0e7dc2ae657b4a01aec85f8de7fdf77.1612796617.git.christophe.leroy@csgroup.eu>
Excerpts from Christophe Leroy's message of February 9, 2021 1:10 am:
> In preparation of porting PPC32 to C syscall entry/exit,
> rewrite the following helpers as static inline functions and
> add support for PPC32 in them:
> __hard_irq_enable()
> __hard_irq_disable()
> __hard_EE_RI_disable()
> __hard_RI_enable()
>
> Then use them in PPC32 version of arch_local_irq_disable()
> and arch_local_irq_enable() to avoid code duplication.
>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/include/asm/hw_irq.h | 75 +++++++++++++++++++++----------
> arch/powerpc/include/asm/reg.h | 1 +
> 2 files changed, 52 insertions(+), 24 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h
> index ed0c3b049dfd..4739f61e632c 100644
> --- a/arch/powerpc/include/asm/hw_irq.h
> +++ b/arch/powerpc/include/asm/hw_irq.h
> @@ -50,6 +50,55 @@
>
> #ifndef __ASSEMBLY__
>
> +static inline void __hard_irq_enable(void)
> +{
> + if (IS_ENABLED(CONFIG_BOOKE) || IS_ENABLED(CONFIG_40x))
> + wrtee(MSR_EE);
> + else if (IS_ENABLED(CONFIG_PPC_8xx))
> + wrtspr(SPRN_EIE);
> + else if (IS_ENABLED(CONFIG_PPC_BOOK3S_64))
> + __mtmsrd(MSR_EE | MSR_RI, 1);
> + else
> + mtmsr(mfmsr() | MSR_EE);
> +}
> +
> +static inline void __hard_irq_disable(void)
> +{
> + if (IS_ENABLED(CONFIG_BOOKE) || IS_ENABLED(CONFIG_40x))
> + wrtee(0);
> + else if (IS_ENABLED(CONFIG_PPC_8xx))
> + wrtspr(SPRN_EID);
> + else if (IS_ENABLED(CONFIG_PPC_BOOK3S_64))
> + __mtmsrd(MSR_RI, 1);
> + else
> + mtmsr(mfmsr() & ~MSR_EE);
> +}
> +
> +static inline void __hard_EE_RI_disable(void)
> +{
> + if (IS_ENABLED(CONFIG_BOOKE) || IS_ENABLED(CONFIG_40x))
> + wrtee(0);
> + else if (IS_ENABLED(CONFIG_PPC_8xx))
> + wrtspr(SPRN_NRI);
> + else if (IS_ENABLED(CONFIG_PPC_BOOK3S_64))
> + __mtmsrd(0, 1);
> + else
> + mtmsr(mfmsr() & ~(MSR_EE | MSR_RI));
> +}
> +
> +static inline void __hard_RI_enable(void)
> +{
> + if (IS_ENABLED(CONFIG_BOOKE) || IS_ENABLED(CONFIG_40x))
> + return;
> +
> + if (IS_ENABLED(CONFIG_PPC_8xx))
> + wrtspr(SPRN_EID);
> + else if (IS_ENABLED(CONFIG_PPC_BOOK3S_64))
> + __mtmsrd(MSR_RI, 1);
> + else
> + mtmsr(mfmsr() | MSR_RI);
> +}
> +
> #ifdef CONFIG_PPC64
> #include <asm/paca.h>
>
> @@ -212,18 +261,6 @@ static inline bool arch_irqs_disabled(void)
>
> #endif /* CONFIG_PPC_BOOK3S */
>
> -#ifdef CONFIG_PPC_BOOK3E
> -#define __hard_irq_enable() wrtee(MSR_EE)
> -#define __hard_irq_disable() wrtee(0)
> -#define __hard_EE_RI_disable() wrtee(0)
> -#define __hard_RI_enable() do { } while (0)
> -#else
> -#define __hard_irq_enable() __mtmsrd(MSR_EE|MSR_RI, 1)
> -#define __hard_irq_disable() __mtmsrd(MSR_RI, 1)
> -#define __hard_EE_RI_disable() __mtmsrd(0, 1)
> -#define __hard_RI_enable() __mtmsrd(MSR_RI, 1)
> -#endif
> -
> #define hard_irq_disable() do { \
> unsigned long flags; \
> __hard_irq_disable(); \
> @@ -322,22 +359,12 @@ static inline unsigned long arch_local_irq_save(void)
>
> static inline void arch_local_irq_disable(void)
> {
> - if (IS_ENABLED(CONFIG_BOOKE))
> - wrtee(0);
> - else if (IS_ENABLED(CONFIG_PPC_8xx))
> - wrtspr(SPRN_EID);
> - else
> - mtmsr(mfmsr() & ~MSR_EE);
> + __hard_irq_disable();
> }
>
> static inline void arch_local_irq_enable(void)
> {
> - if (IS_ENABLED(CONFIG_BOOKE))
> - wrtee(MSR_EE);
> - else if (IS_ENABLED(CONFIG_PPC_8xx))
> - wrtspr(SPRN_EIE);
> - else
> - mtmsr(mfmsr() | MSR_EE);
> + __hard_irq_enable();
> }
>
> static inline bool arch_irqs_disabled_flags(unsigned long flags)
> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> index c5a3e856191c..bc4305ba00d0 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
> @@ -1375,6 +1375,7 @@
> #define mtmsr(v) asm volatile("mtmsr %0" : \
> : "r" ((unsigned long)(v)) \
> : "memory")
> +#define __mtmsrd(v, l) BUILD_BUG()
> #define __MTMSR "mtmsr"
> #endif
>
> --
> 2.25.0
>
>
^ permalink raw reply
* Re: [PATCH v5 05/22] powerpc/irq: Add helper to set regs->softe
From: Nicholas Piggin @ 2021-02-09 1:11 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
msuchanek, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <5f37d1177a751fdbca79df461d283850ca3a34a2.1612796617.git.christophe.leroy@csgroup.eu>
Excerpts from Christophe Leroy's message of February 9, 2021 1:10 am:
> regs->softe doesn't exist on PPC32.
>
> Add irq_soft_mask_regs_set_state() helper to set regs->softe.
> This helper will void on PPC32.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/include/asm/hw_irq.h | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h
> index 614957f74cee..ed0c3b049dfd 100644
> --- a/arch/powerpc/include/asm/hw_irq.h
> +++ b/arch/powerpc/include/asm/hw_irq.h
> @@ -38,6 +38,8 @@
> #define PACA_IRQ_MUST_HARD_MASK (PACA_IRQ_EE)
> #endif
>
> +#endif /* CONFIG_PPC64 */
> +
> /*
> * flags for paca->irq_soft_mask
> */
> @@ -46,8 +48,6 @@
> #define IRQS_PMI_DISABLED 2
> #define IRQS_ALL_DISABLED (IRQS_DISABLED | IRQS_PMI_DISABLED)
>
> -#endif /* CONFIG_PPC64 */
> -
> #ifndef __ASSEMBLY__
>
> #ifdef CONFIG_PPC64
> @@ -287,6 +287,10 @@ extern void irq_set_pending_from_srr1(unsigned long srr1);
>
> extern void force_external_irq_replay(void);
>
> +static inline void irq_soft_mask_regs_set_state(struct pt_regs *regs, unsigned long val)
> +{
> + regs->softe = val;
> +}
> #else /* CONFIG_PPC64 */
>
> static inline unsigned long arch_local_save_flags(void)
> @@ -355,6 +359,9 @@ static inline bool arch_irq_disabled_regs(struct pt_regs *regs)
>
> static inline void may_hard_irq_enable(void) { }
>
> +static inline void irq_soft_mask_regs_set_state(struct pt_regs *regs, unsigned long val)
> +{
> +}
> #endif /* CONFIG_PPC64 */
>
> #define ARCH_IRQ_INIT_FLAGS IRQ_NOREQUEST
What I don't like about this where you use it is it kind of pollutes
the ppc32 path with this function which is not valid to use.
I would prefer if you had this purely so it could compile with:
if (IS_ENABLED(CONFIG_PPC64)))
irq_soft_mask_regs_set_state(regs, blah);
And then you could make the ppc32 cause a link error if it did not
get eliminated at compile time (e.g., call an undefined function).
You could do the same with the kuap_ functions to change some ifdefs
to IS_ENABLED.
That's just my preference but if you prefer this way I guess that's
okay.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v5 00/22] powerpc/32: Implement C syscall entry/exit
From: Nicholas Piggin @ 2021-02-09 1:03 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
msuchanek, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1612796617.git.christophe.leroy@csgroup.eu>
Excerpts from Christophe Leroy's message of February 9, 2021 1:10 am:
> This series implements C syscall entry/exit for PPC32. It reuses
> the work already done for PPC64.
>
> This series is based on today's merge-test (b6f72fc05389e3fc694bf5a5fa1bbd33f61879e0)
>
> In terms on performance we have the following number of cycles on an
> 8xx running null_syscall benchmark:
> - mainline: 296 cycles
> - after patch 4: 283 cycles
> - after patch 16: 304 cycles
> - after patch 17: 348 cycles
> - at the end of the series: 320 cycles
>
> So in summary, we have a degradation of performance of 8% on null_syscall.
>
> I think it is not a big degradation, it is worth it.
I guess it's 13% from 283. But it's very nice to use the shared C code.
There might be a few more percent speedup in there we can find later.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v2 2/2] memblock: do not start bottom-up allocations with kernel_end
From: Thiago Jung Bauermann @ 2021-02-08 23:58 UTC (permalink / raw)
To: Mike Rapoport
Cc: riel, iamjoonsoo.kim, Ram Pai, linux-kernel, mhocko, linux-mm,
Satheesh Rajendran, guro, Konrad Rzeszutek Wilk, Andrew Morton,
linuxppc-dev, kernel-team
In-Reply-To: <20210124073421.GG6332@kernel.org>
Mike Rapoport <rppt@kernel.org> writes:
> On Sat, Jan 23, 2021 at 06:09:11PM -0800, Andrew Morton wrote:
>> On Fri, 22 Jan 2021 01:37:14 -0300 Thiago Jung Bauermann <bauerman@linux.ibm.com> wrote:
>>
>> > Mike Rapoport <rppt@kernel.org> writes:
>> >
>> > > > Signed-off-by: Roman Gushchin <guro@fb.com>
>> > >
>> > > Reviewed-by: Mike Rapoport <rppt@linux.ibm.com>
>> >
>> > I've seen a couple of spurious triggers of the WARN_ONCE() removed by this
>> > patch. This happens on some ppc64le bare metal (powernv) server machines with
>> > CONFIG_SWIOTLB=y and crashkernel=4G, as described in a candidate patch I posted
>> > to solve this issue in a different way:
>> >
>> > https://lore.kernel.org/linuxppc-dev/20201218062103.76102-1-bauerman@linux.ibm.com/
>> >
>> > Since this patch solves that problem, is it possible to include it in the next
>> > feasible v5.11-rcX, with the following tag?
>>
>> We could do this, if we're confident that this patch doesn't depend on
>> [1/2] "mm: cma: allocate cma areas bottom-up"? I think it is...
>
> A think it does not depend on cma bottom-up allocation, it's rather the other
> way around: without this CMA bottom-up allocation could fail with KASLR
> enabled.
I noticed that this patch is now upstream as:
2dcb39645441 memblock: do not start bottom-up allocations with kernel_end
> Still, this patch may need updates to the way x86 does early reservations:
>
> https://lore.kernel.org/lkml/20210115083255.12744-1-rppt@kernel.org
... but the patches from this link still aren't. Isn't this a potential
problem for x86?
The patch series on the link above is now superseded by v2:
https://lore.kernel.org/linux-mm/20210128105711.10428-1-rppt@kernel.org/
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* [PATCH v5 05/10] powerpc: dts: akebono: Harmonize EHCI/OHCI DT nodes name
From: Serge Semin @ 2021-02-08 13:51 UTC (permalink / raw)
To: Felipe Balbi, Bjorn Andersson, Krzysztof Kozlowski,
Florian Fainelli, Rob Herring, Greg Kroah-Hartman,
Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras
Cc: Serge Semin, devicetree, linuxppc-dev, linux-kernel, Serge Semin
In-Reply-To: <20210208135154.6645-1-Sergey.Semin@baikalelectronics.ru>
In accordance with the Generic EHCI/OHCI bindings the corresponding node
name is suppose to comply with the Generic USB HCD DT schema, which
requires the USB nodes to have the name acceptable by the regexp:
"^usb(@.*)?" . Make sure the "generic-ehci" and "generic-ohci"-compatible
nodes are correctly named.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Acked-by: Krzysztof Kozlowski <krzk@kernel.org>
---
arch/powerpc/boot/dts/akebono.dts | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/boot/dts/akebono.dts b/arch/powerpc/boot/dts/akebono.dts
index df18f8dc4642..343326c30380 100644
--- a/arch/powerpc/boot/dts/akebono.dts
+++ b/arch/powerpc/boot/dts/akebono.dts
@@ -126,7 +126,7 @@ SATA0: sata@30000010000 {
interrupts = <93 2>;
};
- EHCI0: ehci@30010000000 {
+ EHCI0: usb@30010000000 {
compatible = "ibm,476gtr-ehci", "generic-ehci";
reg = <0x300 0x10000000 0x0 0x10000>;
interrupt-parent = <&MPIC>;
@@ -140,14 +140,14 @@ SD0: sd@30000000000 {
interrupt-parent = <&MPIC>;
};
- OHCI0: ohci@30010010000 {
+ OHCI0: usb@30010010000 {
compatible = "ibm,476gtr-ohci", "generic-ohci";
reg = <0x300 0x10010000 0x0 0x10000>;
interrupt-parent = <&MPIC>;
interrupts = <89 1>;
};
- OHCI1: ohci@30010020000 {
+ OHCI1: usb@30010020000 {
compatible = "ibm,476gtr-ohci", "generic-ohci";
reg = <0x300 0x10020000 0x0 0x10000>;
interrupt-parent = <&MPIC>;
--
2.29.2
^ permalink raw reply related
* [PATCH v5 00/10] dt-bindings: usb: Harmonize xHCI/EHCI/OHCI/DWC3 nodes name
From: Serge Semin @ 2021-02-08 13:51 UTC (permalink / raw)
To: Felipe Balbi, Bjorn Andersson, Krzysztof Kozlowski,
Florian Fainelli, Rob Herring, Greg Kroah-Hartman
Cc: Andrew Lunn, Amelie Delaunay, Tony Lindgren, linux-mips,
Paul Cercueil, Paul Mackerras, linux-stm32, linux-kernel,
Alexandre Torgue, Khuong Dinh, linux-samsung-soc, Gregory Clement,
Rafal Milecki, Alexey Brodkin, Wei Xu, Chen-Yu Tsai, Andy Gross,
bcm-kernel-feedback-list, linux-arm-msm, linux-snps-arc,
Sebastian Hesselbarth, devicetree, Jason Cooper, Hauke Mehrtens,
linuxppc-dev, Maxime Ripard, Vladimir Zapolskiy, Jun Li,
Santosh Shilimkar, Matthias Brugger, Benoit Cousson, linux-omap,
linux-arm-kernel, Thomas Bogendoerfer, Vineet Gupta,
Patrice Chotard, Serge Semin, Li Yang, Serge Semin, Kukjin Kim,
Maxime Coquelin, linux-mediatek, Shawn Guo
As the subject states this series is an attempt to harmonize the xHCI,
EHCI, OHCI and DWC USB3 DT nodes with the DT schema introduced in the
framework of the patchset [1].
Firstly as Krzysztof suggested we've deprecated a support of DWC USB3
controllers with "synopsys,"-vendor prefix compatible string in favor of
the ones with valid "snps,"-prefix. It's done in all the DTS files,
which have been unfortunate to define such nodes.
Secondly we suggest to fix the snps,quirk-frame-length-adjustment property
declaration in the Amlogic meson-g12-common.dtsi DTS file, since it has
been erroneously declared as boolean while having uint32 type. Neil said
it was ok to init that property with 0x20 value.
Thirdly the main part of the patchset concern fixing the xHCI, EHCI/OHCI
and DWC USB3 DT nodes name as in accordance with their DT schema the
corresponding node name is suppose to comply with the Generic USB HCD DT
schema, which requires the USB nodes to have the name acceptable by the
regexp: "^usb(@.*)?". Such requirement had been applicable even before we
introduced the new DT schema in [1], but as we can see it hasn't been
strictly implemented for a lot the DTS files. Since DT schema is now
available the automated DTS validation shall make sure that the rule isn't
violated.
Note most of these patches have been a part of the last three patches of
[1]. But since there is no way to have them merged in in a combined
manner, I had to move them to the dedicated series and split them up so to
be accepted by the corresponding subsystem maintainers one-by-one.
[1] Link: https://lore.kernel.org/linux-usb/20201014101402.18271-1-Sergey.Semin@baikalelectronics.ru/
Changelog v1:
- As Krzysztof suggested I've created a script which checked whether the
node names had been also updated in all the depended dts files. As a
result I found two more files which should have been also modified:
arch/arc/boot/dts/{axc003.dtsi,axc003_idu.dtsi}
- Correct the USB DWC3 nodes name found in
arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} too.
Link: https://lore.kernel.org/linux-usb/20201020115959.2658-1-Sergey.Semin@baikalelectronics.ru
Changelog v2:
- Drop the patch:
[PATCH 01/29] usb: dwc3: Discard synopsys,dwc3 compatibility string
and get back the one which marks the "synopsys,dwc3" compatible string
as deprecated into the DT schema related series.
- Drop the patches:
[PATCH 03/29] arm: dts: am437x: Correct DWC USB3 compatible string
[PATCH 04/29] arm: dts: exynos: Correct DWC USB3 compatible string
[PATCH 07/29] arm: dts: bcm53x: Harmonize EHCI/OHCI DT nodes name
[PATCH 08/29] arm: dts: stm32: Harmonize EHCI/OHCI DT nodes name
[PATCH 16/29] arm: dts: bcm5301x: Harmonize xHCI DT nodes name
[PATCH 19/29] arm: dts: exynos: Harmonize DWC USB3 DT nodes name
[PATCH 21/29] arm: dts: ls1021a: Harmonize DWC USB3 DT nodes name
[PATCH 22/29] arm: dts: omap5: Harmonize DWC USB3 DT nodes name
[PATCH 24/29] arm64: dts: allwinner: h6: Harmonize DWC USB3 DT nodes name
[PATCH 26/29] arm64: dts: exynos: Harmonize DWC USB3 DT nodes name
[PATCH 27/29] arm64: dts: layerscape: Harmonize DWC USB3 DT nodes name
since they have been applied to the corresponding maintainers repos.
- Fix drivers/usb/dwc3/dwc3-qcom.c to be looking for the "usb@"-prefixed
sub-node and falling back to the "dwc3@"-prefixed one on failure.
Link: https://lore.kernel.org/linux-usb/20201111091552.15593-1-Sergey.Semin@baikalelectronics.ru
Changelog v3:
- Drop the patches:
[PATCH v2 04/18] arm: dts: hisi-x5hd2: Harmonize EHCI/OHCI DT nodes name
[PATCH v2 06/18] arm64: dts: hisi: Harmonize EHCI/OHCI DT nodes name
[PATCH v2 07/18] mips: dts: jz47x: Harmonize EHCI/OHCI DT nodes name
[PATCH v2 08/18] mips: dts: sead3: Harmonize EHCI/OHCI DT nodes name
[PATCH v2 09/18] mips: dts: ralink: mt7628a: Harmonize EHCI/OHCI DT nodes name
[PATCH v2 11/18] arm64: dts: marvell: cp11x: Harmonize xHCI DT nodes name
[PATCH v2 12/18] arm: dts: marvell: armada-375: Harmonize DWC USB3 DT nodes name
[PATCH v2 16/18] arm64: dts: hi3660: Harmonize DWC USB3 DT nodes name
since they have been applied to the corresponding maintainers repos.
Link: https://lore.kernel.org/linux-usb/20201205155621.3045-1-Sergey.Semin@baikalelectronics.ru
Changelog v4:
- Just resend.
Link: https://lore.kernel.org/linux-usb/20201210091756.18057-1-Sergey.Semin@baikalelectronics.ru/
Changelog v5:
- Drop the patch:
[PATCH v4 02/10] arm64: dts: amlogic: meson-g12: Set FL-adj property value
since it has been applied to the corresponding maintainers repos.
- Get back the patch:
[PATCH 21/29] arm: dts: ls1021a: Harmonize DWC USB3 DT nodes name
as it has been missing in the kernel 5.11-rc7
- Rebase onto the kernel 5.11-rc7
Cc: Vineet Gupta <vgupta@synopsys.com>
Cc: Rafal Milecki <zajec5@gmail.com>
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Santosh Shilimkar <ssantosh@kernel.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Benoit Cousson <bcousson@baylibre.com>
Cc: Patrice Chotard <patrice.chotard@st.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Khuong Dinh <khuong@os.amperecomputing.com>
Cc: Andy Gross <agross@kernel.org>
Cc: Alexey Brodkin <abrodkin@synopsys.com>
Cc: Hauke Mehrtens <hauke@hauke-m.de>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Cc: Amelie Delaunay <amelie.delaunay@st.com>
Cc: Vladimir Zapolskiy <vz@mleia.com>
Cc: Paul Cercueil <paul@crapouillou.net>
Cc: Matthias Brugger <matthias.bgg@gmail.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@bootlin.com>
Cc: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Cc: Kukjin Kim <kgene@kernel.org>
Cc: Li Yang <leoyang.li@nxp.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Jun Li <lijun.kernel@gmail.com>
Cc: linux-snps-arc@lists.infradead.org
Cc: bcm-kernel-feedback-list@broadcom.com
Cc: linux-stm32@st-md-mailman.stormreply.com
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-mips@vger.kernel.org
Cc: linux-mediatek@lists.infradead.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-samsung-soc@vger.kernel.org
Cc: linux-omap@vger.kernel.org
Cc: linux-arm-msm@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Serge Semin (10):
arm: dts: ls1021a: Harmonize DWC USB3 DT nodes name
arm: dts: keystone: Correct DWC USB3 compatible string
arc: dts: Harmonize EHCI/OHCI DT nodes name
arm: dts: lpc18xx: Harmonize EHCI/OHCI DT nodes name
powerpc: dts: akebono: Harmonize EHCI/OHCI DT nodes name
arm: dts: keystone: Harmonize DWC USB3 DT nodes name
arm: dts: stih407-family: Harmonize DWC USB3 DT nodes name
arm64: dts: apm: Harmonize DWC USB3 DT nodes name
usb: dwc3: qcom: Detect DWC3 DT-nodes with "usb"-prefixed names
arm64: dts: qcom: Harmonize DWC USB3 DT nodes name
arch/arc/boot/dts/axc003.dtsi | 4 ++--
arch/arc/boot/dts/axc003_idu.dtsi | 4 ++--
arch/arc/boot/dts/axs10x_mb.dtsi | 4 ++--
arch/arc/boot/dts/hsdk.dts | 4 ++--
arch/arc/boot/dts/vdk_axs10x_mb.dtsi | 2 +-
arch/arm/boot/dts/keystone-k2e.dtsi | 6 +++---
arch/arm/boot/dts/keystone.dtsi | 4 ++--
arch/arm/boot/dts/lpc18xx.dtsi | 4 ++--
arch/arm/boot/dts/ls1021a.dtsi | 2 +-
arch/arm/boot/dts/stih407-family.dtsi | 2 +-
arch/arm64/boot/dts/apm/apm-shadowcat.dtsi | 4 ++--
arch/arm64/boot/dts/apm/apm-storm.dtsi | 6 +++---
arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi | 4 ++--
arch/arm64/boot/dts/qcom/ipq8074.dtsi | 4 ++--
arch/arm64/boot/dts/qcom/msm8996.dtsi | 4 ++--
arch/arm64/boot/dts/qcom/msm8998.dtsi | 2 +-
arch/arm64/boot/dts/qcom/qcs404-evb.dtsi | 2 +-
arch/arm64/boot/dts/qcom/qcs404.dtsi | 4 ++--
arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 +-
arch/arm64/boot/dts/qcom/sdm845.dtsi | 4 ++--
arch/arm64/boot/dts/qcom/sm8150.dtsi | 2 +-
arch/powerpc/boot/dts/akebono.dts | 6 +++---
drivers/usb/dwc3/dwc3-qcom.c | 3 ++-
23 files changed, 42 insertions(+), 41 deletions(-)
--
2.29.2
^ permalink raw reply
* Re: [PATCH] ASoC: fsl: constify static snd_soc_dai_ops structs
From: Mark Brown @ 2021-02-08 18:38 UTC (permalink / raw)
To: Timur Tabi, Jaroslav Kysela, Rikard Falkeborn, Takashi Iwai,
Nicolin Chen, Xiubo Li
Cc: alsa-devel, Shengjiu Wang, linux-kernel, Liam Girdwood,
Fabio Estevam, linuxppc-dev
In-Reply-To: <20210206225849.51071-1-rikard.falkeborn@gmail.com>
On Sat, 6 Feb 2021 23:58:49 +0100, Rikard Falkeborn wrote:
> The only usage of these is to assign their address to the 'ops' field in
> the snd_soc_dai_driver struct, which is a pointer to const. Make them
> const to allow the compiler to put them in read-only memory.
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/1] ASoC: fsl: constify static snd_soc_dai_ops structs
commit: 38d89a564847048c0f6fe53a829d15edb4f21da3
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply
* Re: [PATCH v4 11/23] powerpc/syscall: Rename syscall_64.c into syscall.c
From: Christophe Leroy @ 2021-02-08 17:47 UTC (permalink / raw)
To: Nicholas Piggin, Benjamin Herrenschmidt, Michael Ellerman,
msuchanek, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1611656343.yaxha7r2q4.astroid@bobo.none>
Le 26/01/2021 à 11:21, Nicholas Piggin a écrit :
> Excerpts from Christophe Leroy's message of January 26, 2021 12:48 am:
>> syscall_64.c will be reused almost as is for PPC32.
>>
>> Rename it syscall.c
>
> Could you rename it to interrupt.c instead? A system call is an
> interrupt, and the file now also has code to return from other
> interrupts as well, and it matches the new asm/interrupt.h from
> the interrupts series.
>
Done in v5
Christophe
^ permalink raw reply
* Re: [PATCH v4 14/23] powerpc/syscall: Save r3 in regs->orig_r3
From: Christophe Leroy @ 2021-02-08 17:47 UTC (permalink / raw)
To: Nicholas Piggin, Benjamin Herrenschmidt, Michael Ellerman,
msuchanek, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1611656262.d3l09kg9o2.astroid@bobo.none>
Le 26/01/2021 à 11:18, Nicholas Piggin a écrit :
> Excerpts from Christophe Leroy's message of January 26, 2021 12:48 am:
>> Save r3 in regs->orig_r3 in system_call_exception()
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>> arch/powerpc/kernel/entry_64.S | 1 -
>> arch/powerpc/kernel/syscall.c | 2 ++
>> 2 files changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
>> index aa1af139d947..a562a4240aa6 100644
>> --- a/arch/powerpc/kernel/entry_64.S
>> +++ b/arch/powerpc/kernel/entry_64.S
>> @@ -278,7 +278,6 @@ END_BTB_FLUSH_SECTION
>> std r10,_LINK(r1)
>> std r11,_TRAP(r1)
>> std r12,_CCR(r1)
>> - std r3,ORIG_GPR3(r1)
>> addi r10,r1,STACK_FRAME_OVERHEAD
>> ld r11,exception_marker@toc(r2)
>> std r11,-16(r10) /* "regshere" marker */
>
> This misses system_call_vectored.
Oops yes, this patch was cooked before SCV where introduced. Fixes in v5.
Thanks
Christophe
^ permalink raw reply
* Re: [PATCH v4 20/23] powerpc/syscall: Do not check unsupported scv vector on PPC32
From: Christophe Leroy @ 2021-02-08 17:45 UTC (permalink / raw)
To: Nicholas Piggin, Benjamin Herrenschmidt, Michael Ellerman,
msuchanek, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1611656145.efq1cxcpts.astroid@bobo.none>
Le 26/01/2021 à 11:16, Nicholas Piggin a écrit :
> Excerpts from Christophe Leroy's message of January 26, 2021 12:48 am:
>> Only PPC64 has scv. No need to check the 0x7ff0 trap on PPC32.
>>
>> And ignore the scv parameter in syscall_exit_prepare (Save 14 cycles
>> 346 => 332 cycles)
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>> arch/powerpc/kernel/entry_32.S | 1 -
>> arch/powerpc/kernel/syscall.c | 7 +++++--
>> 2 files changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
>> index 9922a04650f7..6ae9c7bcb06c 100644
>> --- a/arch/powerpc/kernel/entry_32.S
>> +++ b/arch/powerpc/kernel/entry_32.S
>> @@ -343,7 +343,6 @@ transfer_to_syscall:
>>
>> ret_from_syscall:
>> addi r4,r1,STACK_FRAME_OVERHEAD
>> - li r5,0
>> bl syscall_exit_prepare
>> #if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
>> /* If the process has its own DBCR0 value, load it up. The internal
>> diff --git a/arch/powerpc/kernel/syscall.c b/arch/powerpc/kernel/syscall.c
>> index 476909b11051..30f8a397a522 100644
>> --- a/arch/powerpc/kernel/syscall.c
>> +++ b/arch/powerpc/kernel/syscall.c
>> @@ -86,7 +86,7 @@ notrace long system_call_exception(long r3, long r4, long r5,
>> local_irq_enable();
>>
>> if (unlikely(current_thread_info()->flags & _TIF_SYSCALL_DOTRACE)) {
>> - if (unlikely(regs->trap == 0x7ff0)) {
>> + if (IS_ENABLED(CONFIG_PPC64) && unlikely(regs->trap == 0x7ff0)) {
>> /* Unsupported scv vector */
>> _exception(SIGILL, regs, ILL_ILLOPC, regs->nip);
>> return regs->gpr[3];
>> @@ -109,7 +109,7 @@ notrace long system_call_exception(long r3, long r4, long r5,
>> r8 = regs->gpr[8];
>>
>> } else if (unlikely(r0 >= NR_syscalls)) {
>> - if (unlikely(regs->trap == 0x7ff0)) {
>> + if (IS_ENABLED(CONFIG_PPC64) && unlikely(regs->trap == 0x7ff0)) {
>
> Perhaps this could be hidden behind a function like trap_is_scv()?
>
> trap_is_unsupported_scv() ?
>
Ok, I did that in v5
Thanks
Christophe
^ permalink raw reply
* Re: [RFC PATCH 1/3] powerpc/lib: implement strlen() in assembly for PPC64
From: Christophe Leroy @ 2021-02-08 17:23 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, wei.guo.simon, segher
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <a272622d3444a1787a0fc4bf61e9192fb0c3cd64.1530780629.git.christophe.leroy@c-s.fr>
Le 05/07/2018 à 10:53, Christophe Leroy a écrit :
> The generic implementation of strlen() reads strings byte per byte.
>
> This patch implements strlen() in assembly based on a read of entire
> words, in the same spirit as what some other arches and glibc do.
>
> strlen() selftest on an XXXXXXXX provides the following values:
>
> Before the patch (ie with the generic strlen() in lib/string.c):
>
> After the patch:
I think we should implement it in C using word-at-a-time
Christophe
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> This serie applies on top of the PPC32 strlen optimisation serie
>
> Untested
>
> arch/powerpc/include/asm/string.h | 3 +-
> arch/powerpc/lib/Makefile | 4 +-
> arch/powerpc/lib/strlen_64.S | 88 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 91 insertions(+), 4 deletions(-)
> create mode 100644 arch/powerpc/lib/strlen_64.S
>
> diff --git a/arch/powerpc/include/asm/string.h b/arch/powerpc/include/asm/string.h
> index 1647de15a31e..8fdcb532de72 100644
> --- a/arch/powerpc/include/asm/string.h
> +++ b/arch/powerpc/include/asm/string.h
> @@ -13,6 +13,7 @@
> #define __HAVE_ARCH_MEMCHR
> #define __HAVE_ARCH_MEMSET16
> #define __HAVE_ARCH_MEMCPY_FLUSHCACHE
> +#define __HAVE_ARCH_STRLEN
>
> extern char * strcpy(char *,const char *);
> extern char * strncpy(char *,const char *, __kernel_size_t);
> @@ -50,8 +51,6 @@ static inline void *memset64(uint64_t *p, uint64_t v, __kernel_size_t n)
> return __memset64(p, v, n * 8);
> }
> #else
> -#define __HAVE_ARCH_STRLEN
> -
> extern void *memset16(uint16_t *, uint16_t, __kernel_size_t);
> #endif
> #endif /* __KERNEL__ */
> diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
> index 670286808928..93706b4cdbde 100644
> --- a/arch/powerpc/lib/Makefile
> +++ b/arch/powerpc/lib/Makefile
> @@ -12,7 +12,7 @@ CFLAGS_REMOVE_feature-fixups.o = $(CC_FLAGS_FTRACE)
>
> obj-y += string.o alloc.o code-patching.o feature-fixups.o
>
> -obj-$(CONFIG_PPC32) += div64.o copy_32.o crtsavres.o strlen_32.o
> +obj-$(CONFIG_PPC32) += div64.o copy_32.o crtsavres.o
>
> # See corresponding test in arch/powerpc/Makefile
> # 64-bit linker creates .sfpr on demand for final link (vmlinux),
> @@ -33,7 +33,7 @@ obj64-$(CONFIG_ALTIVEC) += vmx-helper.o
> obj64-$(CONFIG_KPROBES_SANITY_TEST) += test_emulate_step.o
>
> obj-y += checksum_$(BITS).o checksum_wrappers.o \
> - string_$(BITS).o memcmp_$(BITS).o
> + string_$(BITS).o memcmp_$(BITS).o strlen_$(BITS).o
>
> obj-y += sstep.o ldstfp.o quad.o
> obj64-y += quad.o
> diff --git a/arch/powerpc/lib/strlen_64.S b/arch/powerpc/lib/strlen_64.S
> new file mode 100644
> index 000000000000..c9704f2b697d
> --- /dev/null
> +++ b/arch/powerpc/lib/strlen_64.S
> @@ -0,0 +1,88 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * strlen() for PPC64
> + *
> + * Copyright (C) 2018 Christophe Leroy CS Systemes d'Information.
> + *
> + * Inspired from glibc implementation
> + */
> +#include <asm/ppc_asm.h>
> +#include <asm/export.h>
> +#include <asm/cache.h>
> +
> + .text
> +
> +/*
> + * Algorithm:
> + *
> + * 1) Given a word 'x', we can test to see if it contains any 0 bytes
> + * by subtracting 0x01010101, and seeing if any of the high bits of each
> + * byte changed from 0 to 1. This works because the least significant
> + * 0 byte must have had no incoming carry (otherwise it's not the least
> + * significant), so it is 0x00 - 0x01 == 0xff. For all other
> + * byte values, either they have the high bit set initially, or when
> + * 1 is subtracted you get a value in the range 0x00-0x7f, none of which
> + * have their high bit set. The expression here is
> + * (x - 0x01010101) & ~x & 0x80808080), which gives 0x00000000 when
> + * there were no 0x00 bytes in the word. You get 0x80 in bytes that
> + * match, but possibly false 0x80 matches in the next more significant
> + * byte to a true match due to carries. For little-endian this is
> + * of no consequence since the least significant match is the one
> + * we're interested in, but big-endian needs method 2 to find which
> + * byte matches.
> + * 2) Given a word 'x', we can test to see _which_ byte was zero by
> + * calculating ~(((x & ~0x80808080) - 0x80808080 - 1) | x | ~0x80808080).
> + * This produces 0x80 in each byte that was zero, and 0x00 in all
> + * the other bytes. The '| ~0x80808080' clears the low 7 bits in each
> + * byte, and the '| x' part ensures that bytes with the high bit set
> + * produce 0x00. The addition will carry into the high bit of each byte
> + * iff that byte had one of its low 7 bits set. We can then just see
> + * which was the most significant bit set and divide by 8 to find how
> + * many to add to the index.
> + * This is from the book 'The PowerPC Compiler Writer's Guide',
> + * by Steve Hoxey, Faraydon Karim, Bill Hay and Hank Warren.
> + */
> +
> +_GLOBAL(strlen)
> + andi. r0, r3, 7
> + lis r7, 0x0101
> + addi r10, r3, -8
> + addic r7, r7, 0x0101 /* r7 = 0x01010101 (lomagic) & clear XER[CA] */
> + rldimi r7, r7, 32, 0 /* r7 = 0x0101010101010101 (lomagic) */
> + rotldi r6, r7, 31 /* r6 = 0x8080808080808080 (himagic) */
> + bne- 3f
> + .balign IFETCH_ALIGN_BYTES
> +1: ldu r9, 8(r10)
> +2: subf r8, r7, r9
> + andc r11, r6, r9
> + and. r8, r8, r11
> + beq+ 1b
> +#ifdef CONFIG_CPU_BIG_ENDIAN
> + andc r8, r9, r6
> + orc r9, r9, r6
> + subfe r8, r6, r8
> + nor r8, r8, r9
> + cntlzd r8, r8
> + subf r3, r3, r10
> + srdi r8, r8, 3
> + add r3, r3, r8
> +#else
> + addi r9, r8, -1
> + addi r10, r10, 7
> + andc r8, r9, r8
> + cntlzd r8, r8
> + subf r3, r3, r10
> + srdi r8, r8, 3
> + subf r3, r8, r3
> +#endif
> + blr
> +
> + /* Missaligned string: make sure bytes before string are seen not 0 */
> +3: xor r10, r10, r0
> + orc r8, r8, r8
> + ldu r9, 8(r10)
> + slwi r0, r0, 3
> + srw r8, r8, r0
> + orc r9, r9, r8
> + b 2b
> +EXPORT_SYMBOL(strlen)
>
^ permalink raw reply
* Re: [PATCH 2/8] powerpc/signal: Add unsafe_copy_{vsx, fpr}_from_user()
From: Christophe Leroy @ 2021-02-08 17:18 UTC (permalink / raw)
To: Christopher M. Riedl, linuxppc-dev
In-Reply-To: <C94BGJXVR3GE.11YGE41549ZVT@geist>
Le 08/02/2021 à 18:14, Christopher M. Riedl a écrit :
> On Sun Feb 7, 2021 at 4:12 AM CST, Christophe Leroy wrote:
>>
>>
>> Le 06/02/2021 à 18:39, Christopher M. Riedl a écrit :
>>> On Sat Feb 6, 2021 at 10:32 AM CST, Christophe Leroy wrote:
>>>>
>>>>
>>>> Le 20/10/2020 à 04:01, Christopher M. Riedl a écrit :
>>>>> On Fri Oct 16, 2020 at 10:48 AM CDT, Christophe Leroy wrote:
>>>>>>
>>>>>>
>>>>>> Le 15/10/2020 à 17:01, Christopher M. Riedl a écrit :
>>>>>>> Reuse the "safe" implementation from signal.c except for calling
>>>>>>> unsafe_copy_from_user() to copy into a local buffer. Unlike the
>>>>>>> unsafe_copy_{vsx,fpr}_to_user() functions the "copy from" functions
>>>>>>> cannot use unsafe_get_user() directly to bypass the local buffer since
>>>>>>> doing so significantly reduces signal handling performance.
>>>>>>
>>>>>> Why can't the functions use unsafe_get_user(), why does it significantly
>>>>>> reduces signal handling
>>>>>> performance ? How much significant ? I would expect that not going
>>>>>> through an intermediate memory
>>>>>> area would be more efficient
>>>>>>
>>>>>
>>>>> Here is a comparison, 'unsafe-signal64-regs' avoids the intermediate buffer:
>>>>>
>>>>> | | hash | radix |
>>>>> | -------------------- | ------ | ------ |
>>>>> | linuxppc/next | 289014 | 158408 |
>>>>> | unsafe-signal64 | 298506 | 253053 |
>>>>> | unsafe-signal64-regs | 254898 | 220831 |
>>>>>
>>>>> I have not figured out the 'why' yet. As you mentioned in your series,
>>>>> technically calling __copy_tofrom_user() is overkill for these
>>>>> operations. The only obvious difference between unsafe_put_user() and
>>>>> unsafe_get_user() is that we don't have asm-goto for the 'get' variant.
>>>>> Instead we wrap with unsafe_op_wrap() which inserts a conditional and
>>>>> then goto to the label.
>>>>>
>>>>> Implemenations:
>>>>>
>>>>> #define unsafe_copy_fpr_from_user(task, from, label) do { \
>>>>> struct task_struct *__t = task; \
>>>>> u64 __user *buf = (u64 __user *)from; \
>>>>> int i; \
>>>>> \
>>>>> for (i = 0; i < ELF_NFPREG - 1; i++) \
>>>>> unsafe_get_user(__t->thread.TS_FPR(i), &buf[i], label); \
>>>>> unsafe_get_user(__t->thread.fp_state.fpscr, &buf[i], label); \
>>>>> } while (0)
>>>>>
>>>>> #define unsafe_copy_vsx_from_user(task, from, label) do { \
>>>>> struct task_struct *__t = task; \
>>>>> u64 __user *buf = (u64 __user *)from; \
>>>>> int i; \
>>>>> \
>>>>> for (i = 0; i < ELF_NVSRHALFREG ; i++) \
>>>>> unsafe_get_user(__t->thread.fp_state.fpr[i][TS_VSRLOWOFFSET], \
>>>>> &buf[i], label); \
>>>>> } while (0)
>>>>>
>>>>
>>>> Do you have CONFIG_PROVE_LOCKING or CONFIG_DEBUG_ATOMIC_SLEEP enabled in
>>>> your config ?
>>>
>>> I don't have these set in my config (ppc64le_defconfig). I think I
>>> figured this out - the reason for the lower signal throughput is the
>>> barrier_nospec() in __get_user_nocheck(). When looping we incur that
>>> cost on every iteration. Commenting it out results in signal performance
>>> of ~316K w/ hash on the unsafe-signal64-regs branch. Obviously the
>>> barrier is there for a reason but it is quite costly.
>>
>> Interesting.
>>
>> Can you try with the patch I just sent out
>> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/c72f014730823b413528e90ab6c4d3bcb79f8497.1612692067.git.christophe.leroy@csgroup.eu/
>
> Yeah that patch solves the problem. Using unsafe_get_user() in a loop is
> actually faster on radix than using the intermediary buffer step. A
> summary of results below (unsafe-signal64-v6 uses unsafe_get_user() and
> avoids the local buffer):
>
> | | hash | radix |
> | -------------------------------- | ------ | ------ |
> | unsafe-signal64-v5 | 194533 | 230089 |
> | unsafe-signal64-v6 | 176739 | 202840 |
> | unsafe-signal64-v5+barrier patch | 203037 | 234936 |
> | unsafe-signal64-v6+barrier patch | 205484 | 241030 |
>
> I am still expecting some comments/feedback on my v5 before sending out
> v6. Should I include your patch in my series as well?
>
My patch is now flagged "under review" in patchwork so I suppose Michael picked it already.
Christophe
^ permalink raw reply
* Re: [PATCH 2/8] powerpc/signal: Add unsafe_copy_{vsx,fpr}_from_user()
From: Christopher M. Riedl @ 2021-02-08 17:14 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev
In-Reply-To: <1caa3c1e-bf4e-700e-efea-28964005bb12@csgroup.eu>
On Sun Feb 7, 2021 at 4:12 AM CST, Christophe Leroy wrote:
>
>
> Le 06/02/2021 à 18:39, Christopher M. Riedl a écrit :
> > On Sat Feb 6, 2021 at 10:32 AM CST, Christophe Leroy wrote:
> >>
> >>
> >> Le 20/10/2020 à 04:01, Christopher M. Riedl a écrit :
> >>> On Fri Oct 16, 2020 at 10:48 AM CDT, Christophe Leroy wrote:
> >>>>
> >>>>
> >>>> Le 15/10/2020 à 17:01, Christopher M. Riedl a écrit :
> >>>>> Reuse the "safe" implementation from signal.c except for calling
> >>>>> unsafe_copy_from_user() to copy into a local buffer. Unlike the
> >>>>> unsafe_copy_{vsx,fpr}_to_user() functions the "copy from" functions
> >>>>> cannot use unsafe_get_user() directly to bypass the local buffer since
> >>>>> doing so significantly reduces signal handling performance.
> >>>>
> >>>> Why can't the functions use unsafe_get_user(), why does it significantly
> >>>> reduces signal handling
> >>>> performance ? How much significant ? I would expect that not going
> >>>> through an intermediate memory
> >>>> area would be more efficient
> >>>>
> >>>
> >>> Here is a comparison, 'unsafe-signal64-regs' avoids the intermediate buffer:
> >>>
> >>> | | hash | radix |
> >>> | -------------------- | ------ | ------ |
> >>> | linuxppc/next | 289014 | 158408 |
> >>> | unsafe-signal64 | 298506 | 253053 |
> >>> | unsafe-signal64-regs | 254898 | 220831 |
> >>>
> >>> I have not figured out the 'why' yet. As you mentioned in your series,
> >>> technically calling __copy_tofrom_user() is overkill for these
> >>> operations. The only obvious difference between unsafe_put_user() and
> >>> unsafe_get_user() is that we don't have asm-goto for the 'get' variant.
> >>> Instead we wrap with unsafe_op_wrap() which inserts a conditional and
> >>> then goto to the label.
> >>>
> >>> Implemenations:
> >>>
> >>> #define unsafe_copy_fpr_from_user(task, from, label) do { \
> >>> struct task_struct *__t = task; \
> >>> u64 __user *buf = (u64 __user *)from; \
> >>> int i; \
> >>> \
> >>> for (i = 0; i < ELF_NFPREG - 1; i++) \
> >>> unsafe_get_user(__t->thread.TS_FPR(i), &buf[i], label); \
> >>> unsafe_get_user(__t->thread.fp_state.fpscr, &buf[i], label); \
> >>> } while (0)
> >>>
> >>> #define unsafe_copy_vsx_from_user(task, from, label) do { \
> >>> struct task_struct *__t = task; \
> >>> u64 __user *buf = (u64 __user *)from; \
> >>> int i; \
> >>> \
> >>> for (i = 0; i < ELF_NVSRHALFREG ; i++) \
> >>> unsafe_get_user(__t->thread.fp_state.fpr[i][TS_VSRLOWOFFSET], \
> >>> &buf[i], label); \
> >>> } while (0)
> >>>
> >>
> >> Do you have CONFIG_PROVE_LOCKING or CONFIG_DEBUG_ATOMIC_SLEEP enabled in
> >> your config ?
> >
> > I don't have these set in my config (ppc64le_defconfig). I think I
> > figured this out - the reason for the lower signal throughput is the
> > barrier_nospec() in __get_user_nocheck(). When looping we incur that
> > cost on every iteration. Commenting it out results in signal performance
> > of ~316K w/ hash on the unsafe-signal64-regs branch. Obviously the
> > barrier is there for a reason but it is quite costly.
>
> Interesting.
>
> Can you try with the patch I just sent out
> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/c72f014730823b413528e90ab6c4d3bcb79f8497.1612692067.git.christophe.leroy@csgroup.eu/
Yeah that patch solves the problem. Using unsafe_get_user() in a loop is
actually faster on radix than using the intermediary buffer step. A
summary of results below (unsafe-signal64-v6 uses unsafe_get_user() and
avoids the local buffer):
| | hash | radix |
| -------------------------------- | ------ | ------ |
| unsafe-signal64-v5 | 194533 | 230089 |
| unsafe-signal64-v6 | 176739 | 202840 |
| unsafe-signal64-v5+barrier patch | 203037 | 234936 |
| unsafe-signal64-v6+barrier patch | 205484 | 241030 |
I am still expecting some comments/feedback on my v5 before sending out
v6. Should I include your patch in my series as well?
>
> Thanks
> Christophe
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox