* 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
* Re: [PATCH v2 1/1] powerpc/kvm: Save Timebase Offset to fix sched_clock() while running guest code.
From: Leonardo Bras @ 2021-02-08 16:37 UTC (permalink / raw)
To: Nicholas Piggin, Aneesh Kumar K.V, Athira Rajeev,
Benjamin Herrenschmidt, Christophe Leroy, Frederic Weisbecker,
Geert Uytterhoeven, Jordan Niethe, Michael Ellerman,
Paul Mackerras, Thomas Gleixner
Cc: linuxppc-dev, linux-kernel, kvm-ppc
In-Reply-To: <1612579579.ztbklit4un.astroid@bobo.none>
Hello Nick,
On Sat, 2021-02-06 at 13:03 +1000, Nicholas Piggin wrote:
> Excerpts from Leonardo Bras's message of February 5, 2021 5:01 pm:
> > Hey Nick, thanks for reviewing :)
> >
> > On Fri, 2021-02-05 at 16:28 +1000, Nicholas Piggin wrote:
> > > Excerpts from Leonardo Bras's message of February 5, 2021 4:06 pm:
> > > > Before guest entry, TBU40 register is changed to reflect guest timebase.
> > > > After exitting guest, the register is reverted to it's original value.
> > > >
> > > > If one tries to get the timestamp from host between those changes, it
> > > > will present an incorrect value.
> > > >
> > > > An example would be trying to add a tracepoint in
> > > > kvmppc_guest_entry_inject_int(), which depending on last tracepoint
> > > > acquired could actually cause the host to crash.
> > > >
> > > > Save the Timebase Offset to PACA and use it on sched_clock() to always
> > > > get the correct timestamp.
> > >
> > > Ouch. Not sure how reasonable it is to half switch into guest registers
> > > and expect to call into the wider kernel, fixing things up as we go.
> > > What if mftb is used in other places?
> >
> > IIUC, the CPU is not supposed to call anything as host between guest
> > entry and guest exit, except guest-related cases, like
>
> When I say "call", I'm including tracing in that. If a function is not
> marked as no trace, then it will call into the tracing subsystem.
>
> > kvmppc_guest_entry_inject_int(), but anyway, if something calls mftb it
> > will still get the same value as before.
>
> Right, so it'll be out of whack again.
>
> > This is only supposed to change stuff that depends on sched_clock, like
> > Tracepoints, that can happen in those exceptions.
>
> If they depend on sched_clock that's one thing. Do they definitely have
> no dependencies on mftb from other calls?
We could change that on get_tb() or mftb() @ timebase.h, which would
have a broader reach, but would not reach any mftb from asm code.
> > > Especially as it doesn't seem like there is a reason that function _has_
> > > to be called after the timebase is switched to guest, that's just how
> > > the code is structured.
> >
> > Correct, but if called, like in rb routines, used by tracepoints, the
> > difference between last tb and current (lower) tb may cause the CPU to
> > trap PROGRAM exception, crashing host.
>
> Yes, so I agree with Michael any function that is involved when we begin
> to switch into guest context (or have not completed switching back to
> host going the other way) should be marked as no trace (noinstr even,
> perhaps).
Sure, that would avoid having to get paca->tb_offset for every mftb()
called, and avoid inconsistencies when different ways to get time are
used in code.
On the other hand, it would make it very hard to debug functions like
kvmppc_guest_entry_inject_int() as I am doing right now.
>
> > > As a local hack to work out a bug okay. If you really need it upstream
> > > could you put it under a debug config option?
> >
> > You mean something that is automatically selected whenever those
> > configs are enabled?
> >
> > CONFIG_TRACEPOINT && CONFIG_KVM_BOOK3S_HANDLER && CONFIG_PPC_BOOK3S_64
> >
> > Or something the user need to select himself in menuconfig?
>
> Yeah I meant a default n thing under powerpc kernel debugging somewhere.
So, IIUC all we can do is split this in 2 changes:
1 - Adding notrace to those functions
2 - Introducing a kernel debug config that reverts (1) and 'fixes' mftb
If that's correct, I have some ideas we can use.
For debug option, should we add the offset on get_tb() or mftb()?
Another option would be to adding this tb_offset only in the routines
used by tracing. But this could probably mean having to add a function
in arch-generic code, but still an option.
What do you think?
>
> Thanks,
> Nick
Thank you!
Leonardo Bras
^ permalink raw reply
* [PATCH v5 22/22] powerpc/32: Handle bookE debugging in C in syscall entry/exit
From: Christophe Leroy @ 2021-02-08 15:10 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
msuchanek
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1612796617.git.christophe.leroy@csgroup.eu>
The handling of SPRN_DBCR0 and other registers can easily
be done in C instead of ASM.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
v5: New
---
arch/powerpc/include/asm/reg_booke.h | 3 +++
arch/powerpc/kernel/entry_32.S | 7 -------
arch/powerpc/kernel/head_32.h | 15 --------------
arch/powerpc/kernel/head_booke.h | 19 ------------------
arch/powerpc/kernel/interrupt.c | 29 +++++++++++++++++++++++++++-
5 files changed, 31 insertions(+), 42 deletions(-)
diff --git a/arch/powerpc/include/asm/reg_booke.h b/arch/powerpc/include/asm/reg_booke.h
index 262782f08fd4..17b8dcd9a40d 100644
--- a/arch/powerpc/include/asm/reg_booke.h
+++ b/arch/powerpc/include/asm/reg_booke.h
@@ -691,6 +691,9 @@
#define mttmr(rn, v) asm volatile(MTTMR(rn, %0) : \
: "r" ((unsigned long)(v)) \
: "memory")
+
+extern unsigned long global_dbcr0[];
+
#endif /* !__ASSEMBLY__ */
#endif /* __ASM_POWERPC_REG_BOOKE_H__ */
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index a574201b0eb6..8dea4d3b1d06 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -342,13 +342,6 @@ transfer_to_syscall:
ret_from_syscall:
addi r4,r1,STACK_FRAME_OVERHEAD
bl syscall_exit_prepare
-#if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
- /* If the process has its own DBCR0 value, load it up. The internal
- debug mode bit tells us that dbcr0 should be loaded. */
- lwz r0,THREAD+THREAD_DBCR0(r2)
- andis. r10,r0,DBCR0_IDM@h
- bnel- load_dbcr0
-#endif
#ifdef CONFIG_PPC_47x
lis r4,icache_44x_need_flush@ha
lwz r5,icache_44x_need_flush@l(r4)
diff --git a/arch/powerpc/kernel/head_32.h b/arch/powerpc/kernel/head_32.h
index 5001c6ecc3ec..961b1ce3b6bf 100644
--- a/arch/powerpc/kernel/head_32.h
+++ b/arch/powerpc/kernel/head_32.h
@@ -153,21 +153,6 @@
SAVE_4GPRS(3, r11)
SAVE_2GPRS(7, r11)
addi r2,r12,-THREAD
-#if defined(CONFIG_40x)
- /* Check to see if the dbcr0 register is set up to debug. Use the
- internal debug mode bit to do this. */
- lwz r12,THREAD_DBCR0(r12)
- andis. r12,r12,DBCR0_IDM@h
- beq+ 3f
- /* From user and task is ptraced - load up global dbcr0 */
- li r12,-1 /* clear all pending debug events */
- mtspr SPRN_DBSR,r12
- lis r11,global_dbcr0@ha
- addi r11,r11,global_dbcr0@l
- lwz r12,0(r11)
- mtspr SPRN_DBCR0,r12
-3:
-#endif
b transfer_to_syscall /* jump to handler */
.endm
diff --git a/arch/powerpc/kernel/head_booke.h b/arch/powerpc/kernel/head_booke.h
index 5f565232b99d..47857795f50a 100644
--- a/arch/powerpc/kernel/head_booke.h
+++ b/arch/powerpc/kernel/head_booke.h
@@ -130,25 +130,6 @@ ALT_FTR_SECTION_END_IFSET(CPU_FTR_EMB_HV)
SAVE_2GPRS(7, r11)
addi r2,r10,-THREAD
- /* Check to see if the dbcr0 register is set up to debug. Use the
- internal debug mode bit to do this. */
- lwz r12,THREAD_DBCR0(r10)
- andis. r12,r12,DBCR0_IDM@h
- beq+ 3f
- /* From user and task is ptraced - load up global dbcr0 */
- li r12,-1 /* clear all pending debug events */
- mtspr SPRN_DBSR,r12
- lis r11,global_dbcr0@ha
- addi r11,r11,global_dbcr0@l
-#ifdef CONFIG_SMP
- lwz r10, TASK_CPU(r2)
- slwi r10, r10, 2
- add r11, r11, r10
-#endif
- lwz r12,0(r11)
- mtspr SPRN_DBCR0,r12
-
-3:
b transfer_to_syscall /* jump to handler */
.endm
diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
index c89a8eac3e24..6111acf61373 100644
--- a/arch/powerpc/kernel/interrupt.c
+++ b/arch/powerpc/kernel/interrupt.c
@@ -76,6 +76,13 @@ notrace long system_call_exception(long r3, long r4, long r5,
kuap_check_amr();
#endif
+#ifdef CONFIG_PPC_ADV_DEBUG_REGS
+ if (IS_ENABLED(CONFIG_PPC32) && unlikely(current->thread.debug.dbcr0 & DBCR0_IDM)) {
+ mtspr(SPRN_DBSR, -1);
+ mtspr(SPRN_DBCR0, global_dbcr0[smp_processor_id()]);
+ }
+#endif
+
account_cpu_user_entry();
account_stolen_time();
@@ -324,6 +331,22 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
local_paca->tm_scratch = regs->msr;
#endif
+#ifdef CONFIG_PPC_ADV_DEBUG_REGS
+ if (unlikely(current->thread.debug.dbcr0 & DBCR0_IDM)) {
+ /*
+ * Check to see if the dbcr0 register is set up to debug.
+ * Use the internal debug mode bit to do this.
+ */
+ mtmsr(mfmsr() & ~MSR_DE);
+ if (IS_ENABLED(CONFIG_PPC32)) {
+ isync();
+ global_dbcr0[smp_processor_id()] = mfspr(SPRN_DBCR0);
+ }
+ mtspr(SPRN_DBCR0, current->thread.debug.dbcr0);
+ mtspr(SPRN_DBSR, -1);
+ }
+#endif
+
account_cpu_user_exit();
#ifdef CONFIG_PPC_BOOK3S_64 /* BOOK3E and ppc32 not using this */
@@ -401,13 +424,17 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned
goto again;
}
-#ifdef CONFIG_PPC_BOOK3E
+#ifdef CONFIG_PPC_ADV_DEBUG_REGS
if (unlikely(current->thread.debug.dbcr0 & DBCR0_IDM)) {
/*
* Check to see if the dbcr0 register is set up to debug.
* Use the internal debug mode bit to do this.
*/
mtmsr(mfmsr() & ~MSR_DE);
+ if (IS_ENABLED(CONFIG_PPC32)) {
+ isync();
+ global_dbcr0[smp_processor_id()] = mfspr(SPRN_DBCR0);
+ }
mtspr(SPRN_DBCR0, current->thread.debug.dbcr0);
mtspr(SPRN_DBSR, -1);
}
--
2.25.0
^ permalink raw reply related
* [PATCH v5 21/22] powerpc/32: Remove the counter in global_dbcr0
From: Christophe Leroy @ 2021-02-08 15:10 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
msuchanek
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1612796617.git.christophe.leroy@csgroup.eu>
global_dbcr0 has two parts, 4 bytes to save/restore the
value of SPRN_DBCR0, and 4 bytes that are incremented/decremented
everytime something is saving/loading the above value.
This counter is only incremented/decremented, its value is never
used and never read.
Remove the counter and devide the size of global_dbcr0 by 2.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
v5: New
---
arch/powerpc/kernel/entry_32.S | 12 +++---------
arch/powerpc/kernel/head_32.h | 3 ---
arch/powerpc/kernel/head_booke.h | 5 +----
3 files changed, 4 insertions(+), 16 deletions(-)
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 7c824e8928d0..a574201b0eb6 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -175,14 +175,11 @@ transfer_to_handler:
addi r11,r11,global_dbcr0@l
#ifdef CONFIG_SMP
lwz r9,TASK_CPU(r2)
- slwi r9,r9,3
+ slwi r9,r9,2
add r11,r11,r9
#endif
lwz r12,0(r11)
mtspr SPRN_DBCR0,r12
- lwz r12,4(r11)
- addi r12,r12,-1
- stw r12,4(r11)
#endif
b 3f
@@ -980,14 +977,11 @@ load_dbcr0:
addi r11,r11,global_dbcr0@l
#ifdef CONFIG_SMP
lwz r9,TASK_CPU(r2)
- slwi r9,r9,3
+ slwi r9,r9,2
add r11,r11,r9
#endif
stw r10,0(r11)
mtspr SPRN_DBCR0,r0
- lwz r10,4(r11)
- addi r10,r10,1
- stw r10,4(r11)
li r11,-1
mtspr SPRN_DBSR,r11 /* clear all pending debug events */
blr
@@ -996,7 +990,7 @@ load_dbcr0:
.align 4
.global global_dbcr0
global_dbcr0:
- .space 8*NR_CPUS
+ .space 4*NR_CPUS
.previous
#endif /* !(CONFIG_4xx || CONFIG_BOOKE) */
diff --git a/arch/powerpc/kernel/head_32.h b/arch/powerpc/kernel/head_32.h
index 282d8fd443a9..5001c6ecc3ec 100644
--- a/arch/powerpc/kernel/head_32.h
+++ b/arch/powerpc/kernel/head_32.h
@@ -166,9 +166,6 @@
addi r11,r11,global_dbcr0@l
lwz r12,0(r11)
mtspr SPRN_DBCR0,r12
- lwz r12,4(r11)
- addi r12,r12,-1
- stw r12,4(r11)
3:
#endif
b transfer_to_syscall /* jump to handler */
diff --git a/arch/powerpc/kernel/head_booke.h b/arch/powerpc/kernel/head_booke.h
index bfbd240cc8a2..5f565232b99d 100644
--- a/arch/powerpc/kernel/head_booke.h
+++ b/arch/powerpc/kernel/head_booke.h
@@ -142,14 +142,11 @@ ALT_FTR_SECTION_END_IFSET(CPU_FTR_EMB_HV)
addi r11,r11,global_dbcr0@l
#ifdef CONFIG_SMP
lwz r10, TASK_CPU(r2)
- slwi r10, r10, 3
+ slwi r10, r10, 2
add r11, r11, r10
#endif
lwz r12,0(r11)
mtspr SPRN_DBCR0,r12
- lwz r12,4(r11)
- addi r12,r12,-1
- stw r12,4(r11)
3:
b transfer_to_syscall /* jump to handler */
--
2.25.0
^ permalink raw reply related
* [PATCH v5 20/22] powerpc/syscall: Avoid storing 'current' in another pointer
From: Christophe Leroy @ 2021-02-08 15:10 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
msuchanek
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1612796617.git.christophe.leroy@csgroup.eu>
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>
---
v5: Also in interrupt exit prepare
---
arch/powerpc/kernel/interrupt.c | 28 +++++++++++-----------------
1 file changed, 11 insertions(+), 17 deletions(-)
diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
index 8c38e8c95be2..c89a8eac3e24 100644
--- a/arch/powerpc/kernel/interrupt.c
+++ b/arch/powerpc/kernel/interrupt.c
@@ -223,7 +223,6 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
struct pt_regs *regs,
long scv)
{
- unsigned long *ti_flagsp = ¤t_thread_info()->flags;
unsigned long ti_flags;
unsigned long ret = 0;
@@ -241,7 +240,7 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
/* Check whether the syscall is issued inside a restartable sequence */
rseq_syscall(regs);
- ti_flags = *ti_flagsp;
+ ti_flags = current_thread_info()->flags;
if (unlikely(r3 >= (unsigned long)-MAX_ERRNO) && !scv) {
if (likely(!(ti_flags & (_TIF_NOERROR | _TIF_RESTOREALL)))) {
@@ -255,7 +254,7 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
ret = _TIF_RESTOREALL;
else
regs->gpr[3] = r3;
- clear_bits(_TIF_PERSYSCALL_MASK, ti_flagsp);
+ clear_bits(_TIF_PERSYSCALL_MASK, ¤t_thread_info()->flags);
} else {
regs->gpr[3] = r3;
}
@@ -268,7 +267,7 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
local_irq_disable();
again:
- ti_flags = READ_ONCE(*ti_flagsp);
+ ti_flags = READ_ONCE(current_thread_info()->flags);
while (unlikely(ti_flags & (_TIF_USER_WORK_MASK & ~_TIF_RESTORE_TM))) {
local_irq_enable();
if (ti_flags & _TIF_NEED_RESCHED) {
@@ -284,7 +283,7 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
do_notify_resume(regs, ti_flags);
}
local_irq_disable();
- ti_flags = READ_ONCE(*ti_flagsp);
+ ti_flags = READ_ONCE(current_thread_info()->flags);
}
if (IS_ENABLED(CONFIG_PPC_BOOK3S) && IS_ENABLED(CONFIG_PPC_FPU)) {
@@ -339,10 +338,6 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
#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
- struct thread_struct *ts = ¤t->thread;
-#endif
- unsigned long *ti_flagsp = ¤t_thread_info()->flags;
unsigned long ti_flags;
unsigned long flags;
unsigned long ret = 0;
@@ -365,7 +360,7 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned
local_irq_save(flags);
again:
- ti_flags = READ_ONCE(*ti_flagsp);
+ ti_flags = READ_ONCE(current_thread_info()->flags);
while (unlikely(ti_flags & (_TIF_USER_WORK_MASK & ~_TIF_RESTORE_TM))) {
local_irq_enable(); /* returning to user: may enable */
if (ti_flags & _TIF_NEED_RESCHED) {
@@ -376,7 +371,7 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned
do_notify_resume(regs, ti_flags);
}
local_irq_disable();
- ti_flags = READ_ONCE(*ti_flagsp);
+ ti_flags = READ_ONCE(current_thread_info()->flags);
}
if (IS_ENABLED(CONFIG_PPC_BOOK3S) && IS_ENABLED(CONFIG_PPC_FPU)) {
@@ -407,13 +402,13 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned
}
#ifdef CONFIG_PPC_BOOK3E
- if (unlikely(ts->debug.dbcr0 & DBCR0_IDM)) {
+ if (unlikely(current->thread.debug.dbcr0 & DBCR0_IDM)) {
/*
* Check to see if the dbcr0 register is set up to debug.
* Use the internal debug mode bit to do this.
*/
mtmsr(mfmsr() & ~MSR_DE);
- mtspr(SPRN_DBCR0, ts->debug.dbcr0);
+ mtspr(SPRN_DBCR0, current->thread.debug.dbcr0);
mtspr(SPRN_DBSR, -1);
}
#endif
@@ -438,7 +433,6 @@ void preempt_schedule_irq(void);
notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsigned long msr)
{
- unsigned long *ti_flagsp = ¤t_thread_info()->flags;
unsigned long flags;
unsigned long ret = 0;
#ifdef CONFIG_PPC64
@@ -461,8 +455,8 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign
amr = kuap_get_and_check_amr();
#endif
- if (unlikely(*ti_flagsp & _TIF_EMULATE_STACK_STORE)) {
- clear_bits(_TIF_EMULATE_STACK_STORE, ti_flagsp);
+ if (unlikely(current_thread_info()->flags & _TIF_EMULATE_STACK_STORE)) {
+ clear_bits(_TIF_EMULATE_STACK_STORE, ¤t_thread_info()->flags);
ret = 1;
}
@@ -474,7 +468,7 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign
again:
if (IS_ENABLED(CONFIG_PREEMPT)) {
/* Return to preemptible kernel context */
- if (unlikely(*ti_flagsp & _TIF_NEED_RESCHED)) {
+ if (unlikely(current_thread_info()->flags & _TIF_NEED_RESCHED)) {
if (preempt_count() == 0)
preempt_schedule_irq();
}
--
2.25.0
^ permalink raw reply related
* [PATCH v5 19/22] powerpc/syscall: Optimise checks in beginning of system_call_exception()
From: Christophe Leroy @ 2021-02-08 15:10 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
msuchanek
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1612796617.git.christophe.leroy@csgroup.eu>
Combine all tests of regs->msr into a single logical one.
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 related
* [PATCH v5 18/22] powerpc/syscall: Remove FULL_REGS verification in system_call_exception
From: Christophe Leroy @ 2021-02-08 15:10 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
msuchanek
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1612796617.git.christophe.leroy@csgroup.eu>
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)
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 related
* [PATCH v5 17/22] powerpc/syscall: Do not check unsupported scv vector on PPC32
From: Christophe Leroy @ 2021-02-08 15:10 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
msuchanek
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1612796617.git.christophe.leroy@csgroup.eu>
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);
+}
+
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
#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 related
* [PATCH v5 16/22] powerpc/syscall: Avoid stack frame in likely part of system_call_exception()
From: Christophe Leroy @ 2021-02-08 15:10 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
msuchanek
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1612796617.git.christophe.leroy@csgroup.eu>
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()
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 related
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