* Re: [PATCH] powerpc/mce: Add MCE notification chain
From: Mahesh J Salgaonkar @ 2020-04-06 17:17 UTC (permalink / raw)
To: Nicholas Piggin
Cc: santosh, linuxppc-dev, mahesh, Ganesh, aneesh.kumar, arbab
In-Reply-To: <1586139348.0tunz2vuxz.astroid@bobo.none>
On 2020-04-06 12:17:22 Mon, Nicholas Piggin wrote:
> Ganesh's on April 4, 2020 11:05 pm:
> > On 4/3/20 7:38 AM, Nicholas Piggin wrote:
> >
> >> Ganesh Goudar's on March 30, 2020 5:12 pm:
> >>> From: Santosh S <santosh@fossix.org>
> >>>
> >>> Introduce notification chain which lets know about uncorrected memory
> >>> errors(UE). This would help prospective users in pmem or nvdimm subsystem
> >>> to track bad blocks for better handling of persistent memory allocations.
> >>>
> >>> Signed-off-by: Santosh S <santosh@fossix.org>
> >>> Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
> >> Do you have any such users yet? It would be good to refer to an example
> >> user and give a brief description of what it does in its notifier.
> >
> > Santosh has sent a patch which uses this notification.
> > https://patchwork.ozlabs.org/patch/1265062/
>
> Okay. So these things are asynchronous after the machine check. I guess
> that's the design of it and memory offlining does something similar by
> the looks, but how do you prevent the memory being allocated for
> something else before the notifiers run?
We can't. This race even exists today when we call memory_failure(). If
the same memory is allocated again then we may hit another mce on same
address when touched until the subsystem that has resistered for
notification has completed handling the notified address.
Thanks,
-Mahesh.
>
> >>> @@ -263,6 +277,7 @@ static void machine_process_ue_event(struct work_struct *work)
> >>> while (__this_cpu_read(mce_ue_count) > 0) {
> >>> index = __this_cpu_read(mce_ue_count) - 1;
> >>> evt = this_cpu_ptr(&mce_ue_event_queue[index]);
> >>> + blocking_notifier_call_chain(&mce_notifier_list, 0, evt);
> >> Can we really use a blocking notifier here? I'm not sure that we can.
> >
> > I think we can, do you see any problem?
>
> No it looks okay after better look, sorry for the noise.
>
> Thanks,
> Nick
--
Mahesh J Salgaonkar
^ permalink raw reply
* [PATCH 1/1] powerpc/crash: Use NMI context for printk after crashing other CPUs
From: Leonardo Bras @ 2020-04-06 17:40 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Greg Kroah-Hartman, Enrico Weigelt, Thomas Gleixner,
Allison Randal, Christophe Leroy, Leonardo Bras, Nicholas Piggin
Cc: linuxppc-dev, linux-kernel
Currently, if printk lock (logbuf_lock) is held by other thread during
crash, there is a chance of deadlocking the crash on next printk, and
blocking a possibly desired kdump.
After sending IPI to all other CPUs, make printk enter in NMI context,
as it will use per-cpu buffers to store the message, and avoid locking
logbuf_lock.
Signed-off-by: Leonardo Bras <leonardo@linux.ibm.com>
---
arch/powerpc/kexec/crash.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/kexec/crash.c b/arch/powerpc/kexec/crash.c
index d488311efab1..9b73e3991bf4 100644
--- a/arch/powerpc/kexec/crash.c
+++ b/arch/powerpc/kexec/crash.c
@@ -115,6 +115,7 @@ static void crash_kexec_prepare_cpus(int cpu)
crash_send_ipi(crash_ipi_callback);
smp_wmb();
+ printk_nmi_enter();
again:
/*
--
2.25.1
^ permalink raw reply related
* [RFC PATCH v3 01/15] powerpc/syscall: Refactorise from Nick
From: Christophe Leroy @ 2020-04-06 18:16 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
msuchanek
Cc: linuxppc-dev, linux-kernel
From: Nicholas Piggin <npiggin@gmail.com>
Christophe Leroy's on April 6, 2020 3:44 am:
> ifdef out specific PPC64 stuff to allow building
> syscall_64.c on PPC32.
>
> Modify Makefile to always build syscall.o
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> arch/powerpc/kernel/Makefile | 5 ++---
> arch/powerpc/kernel/syscall.c | 10 ++++++++++
> 2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index 8cc3c831dccd..e4be425b7718 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -45,11 +45,10 @@ obj-y := cputable.o syscalls.o \
> signal.o sysfs.o cacheinfo.o time.o \
> prom.o traps.o setup-common.o \
> udbg.o misc.o io.o misc_$(BITS).o \
> - of_platform.o prom_parse.o
> + of_platform.o prom_parse.o syscall.o
> obj-y += ptrace/
> obj-$(CONFIG_PPC64) += setup_64.o sys_ppc32.o signal_64.o \
> - paca.o nvram_64.o firmware.o note.o \
> - syscall.o
> + paca.o nvram_64.o firmware.o note.o
> obj-$(CONFIG_VDSO32) += vdso32/
> obj-$(CONFIG_PPC_WATCHDOG) += watchdog.o
> obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o
> diff --git a/arch/powerpc/kernel/syscall.c b/arch/powerpc/kernel/syscall.c
> index 72f3d2f0a823..28bd43db8755 100644
> --- a/arch/powerpc/kernel/syscall.c
> +++ b/arch/powerpc/kernel/syscall.c
> @@ -25,8 +25,10 @@ notrace long system_call_exception(long r3, long r4, long r5,
> unsigned long ti_flags;
> syscall_fn f;
>
> +#ifdef CONFIG_PPC64
> if (IS_ENABLED(CONFIG_PPC_IRQ_SOFT_MASK_DEBUG))
> BUG_ON(irq_soft_mask_return() != IRQS_ALL_DISABLED);
> +#endif
>
> trace_hardirqs_off(); /* finish reconciling */
>
> @@ -34,7 +36,9 @@ 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));
> +#ifdef CONFIG_PPC64
> BUG_ON(regs->softe != IRQS_ENABLED);
> +#endif
>
> account_cpu_user_entry();
>
> @@ -56,7 +60,9 @@ 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!
> */
> +#ifdef CONFIG_PPC64
> regs->softe = IRQS_ENABLED;
> +#endif
>
> local_irq_enable();
>
> @@ -148,7 +154,9 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
> ret |= _TIF_RESTOREALL;
> }
>
> +#ifdef CONFIG_PPC64
> again:
> +#endif
> local_irq_disable();
> ti_flags = READ_ONCE(*ti_flagsp);
> while (unlikely(ti_flags & (_TIF_USER_WORK_MASK & ~_TIF_RESTORE_TM))) {
> @@ -191,6 +199,7 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
>
> /* This pattern matches prep_irq_for_idle */
> __hard_EE_RI_disable();
> +#ifdef CONFIG_PPC64
> if (unlikely(lazy_irq_pending())) {
> __hard_RI_enable();
> trace_hardirqs_off();
> @@ -201,6 +210,7 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
> }
> local_paca->irq_happened = 0;
> irq_soft_mask_set(IRQS_ENABLED);
> +#endif
>
> #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> local_paca->tm_scratch = regs->msr;
> --
> 2.25.0
>
>
The #ifdefs disappoint me!
Here is (unested) something that should help 32-bit avoid several ifdefs
in the main part of the function. I should have done this as part of the
merged series, but that's okay I'll submit as a cleanup.
The rest looks okay for now. Maybe we grow some helpers to manage the
soft-mask state, though I'm not really sure it would make sense for
32-bit code to ever call them. Maybe just confined to this file would be
okay but for now the ifdefs are okay.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/kernel/syscall_64.c | 58 +++++++++++++++-----------------
1 file changed, 28 insertions(+), 30 deletions(-)
diff --git a/arch/powerpc/kernel/syscall_64.c b/arch/powerpc/kernel/syscall_64.c
index cf06eb443a80..f021db893ec2 100644
--- a/arch/powerpc/kernel/syscall_64.c
+++ b/arch/powerpc/kernel/syscall_64.c
@@ -103,6 +103,31 @@ notrace long system_call_exception(long r3, long r4, long r5,
return f(r3, r4, r5, r6, r7, r8);
}
+/*
+ * local irqs must be disabled. Returns false if the caller must re-enable
+ * them, check for new work, and try again.
+ */
+static notrace inline bool prep_irq_for_enabled_exit(void)
+{
+ /* This must be done with RI=1 because tracing may touch vmaps */
+ trace_hardirqs_on();
+
+ /* This pattern matches prep_irq_for_idle */
+ __hard_EE_RI_disable();
+ if (unlikely(lazy_irq_pending())) {
+ /* Took an interrupt, may have more exit work to do. */
+ __hard_RI_enable();
+ trace_hardirqs_off();
+ local_paca->irq_happened |= PACA_IRQ_HARD_DIS;
+
+ return false;
+ }
+ local_paca->irq_happened = 0;
+ irq_soft_mask_set(IRQS_ENABLED);
+
+ return true;
+}
+
/*
* This should be called after a syscall returns, with r3 the return value
* from the syscall. If this function returns non-zero, the system call
@@ -186,21 +211,10 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
}
}
- /* This must be done with RI=1 because tracing may touch vmaps */
- trace_hardirqs_on();
-
- /* This pattern matches prep_irq_for_idle */
- __hard_EE_RI_disable();
- if (unlikely(lazy_irq_pending())) {
- __hard_RI_enable();
- trace_hardirqs_off();
- local_paca->irq_happened |= PACA_IRQ_HARD_DIS;
+ if (unlikely(!prep_irq_for_enabled_exit())) {
local_irq_enable();
- /* Took an interrupt, may have more exit work to do. */
goto again;
}
- local_paca->irq_happened = 0;
- irq_soft_mask_set(IRQS_ENABLED);
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
local_paca->tm_scratch = regs->msr;
@@ -264,19 +278,11 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned
}
}
- trace_hardirqs_on();
- __hard_EE_RI_disable();
- if (unlikely(lazy_irq_pending())) {
- __hard_RI_enable();
- trace_hardirqs_off();
- local_paca->irq_happened |= PACA_IRQ_HARD_DIS;
+ if (unlikely(!prep_irq_for_enabled_exit())) {
local_irq_enable();
local_irq_disable();
- /* Took an interrupt, may have more exit work to do. */
goto again;
}
- local_paca->irq_happened = 0;
- irq_soft_mask_set(IRQS_ENABLED);
#ifdef CONFIG_PPC_BOOK3E
if (unlikely(ts->debug.dbcr0 & DBCR0_IDM)) {
@@ -334,13 +340,7 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign
}
}
- trace_hardirqs_on();
- __hard_EE_RI_disable();
- if (unlikely(lazy_irq_pending())) {
- __hard_RI_enable();
- irq_soft_mask_set(IRQS_ALL_DISABLED);
- trace_hardirqs_off();
- local_paca->irq_happened |= PACA_IRQ_HARD_DIS;
+ if (unlikely(!prep_irq_for_enabled_exit())) {
/*
* Can't local_irq_restore to replay if we were in
* interrupt context. Must replay directly.
@@ -354,8 +354,6 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign
/* Took an interrupt, may have more exit work to do. */
goto again;
}
- local_paca->irq_happened = 0;
- irq_soft_mask_set(IRQS_ENABLED);
} else {
/* Returning to a kernel context with local irqs disabled. */
__hard_EE_RI_disable();
--
2.25.0
^ permalink raw reply related
* [RFC PATCH v3 02/15] powerpc/radix: Make kuap_check_amr() and kuap_restore_amr() generic
From: Christophe Leroy @ 2020-04-06 18:16 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
msuchanek
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <5aeaa33383e833f6eca30893fbd188b88e019eaa.1586196948.git.christophe.leroy@c-s.fr>
In preparation of porting powerpc32 to C syscall entry/exit,
rename kuap_check_amr() and kuap_restore_amr() as kuap_check()
and kuap_restore(), and move the stub for when CONFIG_PPC_KUAP is
not selected in the generic asm/kup.h
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/include/asm/book3s/64/kup-radix.h | 12 ++----------
arch/powerpc/include/asm/kup.h | 2 ++
arch/powerpc/kernel/syscall_64.c | 10 +++++-----
3 files changed, 9 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/kup-radix.h b/arch/powerpc/include/asm/book3s/64/kup-radix.h
index 3bcef989a35d..1f2716a0dcd8 100644
--- a/arch/powerpc/include/asm/book3s/64/kup-radix.h
+++ b/arch/powerpc/include/asm/book3s/64/kup-radix.h
@@ -60,13 +60,13 @@
#include <asm/mmu.h>
#include <asm/ptrace.h>
-static inline void kuap_restore_amr(struct pt_regs *regs)
+static inline void kuap_restore(struct pt_regs *regs)
{
if (mmu_has_feature(MMU_FTR_RADIX_KUAP))
mtspr(SPRN_AMR, regs->kuap);
}
-static inline void kuap_check_amr(void)
+static inline void kuap_check(void)
{
if (IS_ENABLED(CONFIG_PPC_KUAP_DEBUG) && mmu_has_feature(MMU_FTR_RADIX_KUAP))
WARN_ON_ONCE(mfspr(SPRN_AMR) != AMR_KUAP_BLOCKED);
@@ -141,14 +141,6 @@ bad_kuap_fault(struct pt_regs *regs, unsigned long address, bool is_write)
(regs->kuap & (is_write ? AMR_KUAP_BLOCK_WRITE : AMR_KUAP_BLOCK_READ)),
"Bug: %s fault blocked by AMR!", is_write ? "Write" : "Read");
}
-#else /* CONFIG_PPC_KUAP */
-static inline void kuap_restore_amr(struct pt_regs *regs)
-{
-}
-
-static inline void kuap_check_amr(void)
-{
-}
#endif /* CONFIG_PPC_KUAP */
#endif /* __ASSEMBLY__ */
diff --git a/arch/powerpc/include/asm/kup.h b/arch/powerpc/include/asm/kup.h
index 92bcd1a26d73..1100c13b6d9e 100644
--- a/arch/powerpc/include/asm/kup.h
+++ b/arch/powerpc/include/asm/kup.h
@@ -62,6 +62,8 @@ bad_kuap_fault(struct pt_regs *regs, unsigned long address, bool is_write)
{
return false;
}
+static inline void kuap_restore(struct pt_regs *regs) { }
+static inline void kuap_check(void) { }
#endif /* CONFIG_PPC_KUAP */
static inline void allow_read_from_user(const void __user *from, unsigned long size)
diff --git a/arch/powerpc/kernel/syscall_64.c b/arch/powerpc/kernel/syscall_64.c
index f021db893ec2..4c46f3aefaf8 100644
--- a/arch/powerpc/kernel/syscall_64.c
+++ b/arch/powerpc/kernel/syscall_64.c
@@ -2,7 +2,7 @@
#include <linux/err.h>
#include <asm/asm-prototypes.h>
-#include <asm/book3s/64/kup-radix.h>
+#include <asm/kup.h>
#include <asm/cputime.h>
#include <asm/hw_irq.h>
#include <asm/kprobes.h>
@@ -48,7 +48,7 @@ notrace long system_call_exception(long r3, long r4, long r5,
}
#endif
- kuap_check_amr();
+ kuap_check();
/*
* This is not required for the syscall exit path, but makes the
@@ -220,7 +220,7 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
local_paca->tm_scratch = regs->msr;
#endif
- kuap_check_amr();
+ kuap_check();
account_cpu_user_exit();
@@ -300,7 +300,7 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned
local_paca->tm_scratch = regs->msr;
#endif
- kuap_check_amr();
+ kuap_check();
account_cpu_user_exit();
@@ -370,7 +370,7 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign
* We don't need to restore AMR on the way back to userspace for KUAP.
* The value of AMR only matters while we're in the kernel.
*/
- kuap_restore_amr(regs);
+ kuap_restore(regs);
return ret;
}
--
2.25.0
^ permalink raw reply related
* [RFC PATCH v3 03/15] powerpc/32s: Create C version of kuap_restore() and kuap_check()
From: Christophe Leroy @ 2020-04-06 18:16 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
msuchanek
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <5aeaa33383e833f6eca30893fbd188b88e019eaa.1586196948.git.christophe.leroy@c-s.fr>
In preparation of porting PPC32 to C syscall entry/exit,
create C version of kuap_restore() and kuap_check() on book3s/32.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/include/asm/book3s/32/kup.h | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/arch/powerpc/include/asm/book3s/32/kup.h b/arch/powerpc/include/asm/book3s/32/kup.h
index 3c0ba22dc360..c85bc5b56366 100644
--- a/arch/powerpc/include/asm/book3s/32/kup.h
+++ b/arch/powerpc/include/asm/book3s/32/kup.h
@@ -102,6 +102,27 @@ static inline void kuap_update_sr(u32 sr, u32 addr, u32 end)
isync(); /* Context sync required after mtsrin() */
}
+static inline void kuap_restore(struct pt_regs *regs)
+{
+ u32 kuap = current->thread.kuap;
+ u32 addr = kuap & 0xf0000000;
+ u32 end = kuap << 28;
+
+ if (unlikely(!kuap))
+ return;
+
+ current->thread.kuap = 0;
+ kuap_update_sr(mfsrin(addr) & ~SR_KS, addr, end); /* Clear Ks */
+}
+
+static inline void kuap_check(void)
+{
+ if (!IS_ENABLED(CONFIG_PPC_KUAP_DEBUG))
+ return;
+
+ WARN_ON_ONCE(current->thread.kuap != 0);
+}
+
static __always_inline void allow_user_access(void __user *to, const void __user *from,
u32 size, unsigned long dir)
{
--
2.25.0
^ permalink raw reply related
* [RFC PATCH v3 04/15] powerpc/8xx: Create C version of kuap_restore() and kuap_check()
From: Christophe Leroy @ 2020-04-06 18:16 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
msuchanek
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <5aeaa33383e833f6eca30893fbd188b88e019eaa.1586196948.git.christophe.leroy@c-s.fr>
In preparation of porting PPC32 to C syscall entry/exit,
create C version of kuap_restore() and kuap_check() on 8xx
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/include/asm/nohash/32/kup-8xx.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/arch/powerpc/include/asm/nohash/32/kup-8xx.h b/arch/powerpc/include/asm/nohash/32/kup-8xx.h
index 85ed2390fb99..1918d2e55da3 100644
--- a/arch/powerpc/include/asm/nohash/32/kup-8xx.h
+++ b/arch/powerpc/include/asm/nohash/32/kup-8xx.h
@@ -34,6 +34,19 @@
#include <asm/reg.h>
+static inline void kuap_restore(struct pt_regs *regs)
+{
+ mtspr(SPRN_MD_AP, regs->kuap);
+}
+
+static inline void kuap_check(void)
+{
+ if (!IS_ENABLED(CONFIG_PPC_KUAP_DEBUG))
+ return;
+
+ WARN_ON_ONCE((mfspr(SPRN_MD_AP) & 0xffff0000) != (MD_APG_KUAP & 0xffff0000));
+}
+
static inline void allow_user_access(void __user *to, const void __user *from,
unsigned long size, unsigned long dir)
{
--
2.25.0
^ permalink raw reply related
* [RFC PATCH v3 05/15] powerpc/irq: Add helpers to get and set regs->softe
From: Christophe Leroy @ 2020-04-06 18:16 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
msuchanek
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <5aeaa33383e833f6eca30893fbd188b88e019eaa.1586196948.git.christophe.leroy@c-s.fr>
regs->softe doesn't exist on PPC32.
Add helpers to get and set regs->softe.
Those helpers will void on PPC32.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/include/asm/hw_irq.h | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h
index e0e71777961f..e69466867d5f 100644
--- a/arch/powerpc/include/asm/hw_irq.h
+++ b/arch/powerpc/include/asm/hw_irq.h
@@ -39,6 +39,8 @@
#define PACA_IRQ_MUST_HARD_MASK (PACA_IRQ_EE)
#endif
+#endif /* CONFIG_PPC64 */
+
/*
* flags for paca->irq_soft_mask
*/
@@ -47,8 +49,6 @@
#define IRQS_PMI_DISABLED 2
#define IRQS_ALL_DISABLED (IRQS_DISABLED | IRQS_PMI_DISABLED)
-#endif /* CONFIG_PPC64 */
-
#ifndef __ASSEMBLY__
extern void replay_system_reset(void);
@@ -282,6 +282,15 @@ extern void irq_set_pending_from_srr1(unsigned long srr1);
extern void force_external_irq_replay(void);
+static inline unsigned long get_softe(struct pt_regs *regs)
+{
+ return regs->softe;
+}
+
+static inline void set_softe(struct pt_regs *regs, unsigned long val)
+{
+ regs->softe = val;
+}
#else /* CONFIG_PPC64 */
static inline unsigned long arch_local_save_flags(void)
@@ -350,6 +359,14 @@ static inline bool arch_irq_disabled_regs(struct pt_regs *regs)
static inline void may_hard_irq_enable(void) { }
+static inline unsigned long get_softe(struct pt_regs *regs)
+{
+ return 0;
+}
+
+static inline void set_softe(struct pt_regs *regs, unsigned long val)
+{
+}
#endif /* CONFIG_PPC64 */
#define ARCH_IRQ_INIT_FLAGS IRQ_NOREQUEST
--
2.25.0
^ permalink raw reply related
* [RFC PATCH v3 06/15] powerpc/irq: Add new helpers to play with MSR_EE and MSR_RI on PPC32
From: Christophe Leroy @ 2020-04-06 18:16 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
msuchanek
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <5aeaa33383e833f6eca30893fbd188b88e019eaa.1586196948.git.christophe.leroy@c-s.fr>
In preparation of porting PPC32 to C syscall entry/exit,
add PPC32 version of following helpers:
__hard_irq_enable()
__hard_irq_disable()
__hard_EE_RI_disable()
__hard_RI_enable()
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/include/asm/hw_irq.h | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h
index e69466867d5f..8c30a72262fd 100644
--- a/arch/powerpc/include/asm/hw_irq.h
+++ b/arch/powerpc/include/asm/hw_irq.h
@@ -330,6 +330,16 @@ static inline void arch_local_irq_disable(void)
mtmsr(mfmsr() & ~MSR_EE);
}
+static inline void arch_local_recovery_disable(void)
+{
+ if (IS_ENABLED(CONFIG_BOOKE))
+ wrtee(0);
+ else if (IS_ENABLED(CONFIG_PPC_8xx))
+ wrtspr(SPRN_NRI);
+ else
+ mtmsr(mfmsr() & ~(MSR_EE | MSR_RI));
+}
+
static inline void arch_local_irq_enable(void)
{
if (IS_ENABLED(CONFIG_BOOKE))
@@ -352,6 +362,11 @@ static inline bool arch_irqs_disabled(void)
#define hard_irq_disable() arch_local_irq_disable()
+#define __hard_irq_enable() arch_local_irq_enable()
+#define __hard_irq_disable() arch_local_irq_disable()
+#define __hard_EE_RI_disable() arch_local_recovery_disable()
+#define __hard_RI_enable() arch_local_irq_disable()
+
static inline bool arch_irq_disabled_regs(struct pt_regs *regs)
{
return !(regs->msr & MSR_EE);
--
2.25.0
^ permalink raw reply related
* [RFC PATCH v3 08/15] powerpc/syscall: Rename syscall_64.c into syscall.c
From: Christophe Leroy @ 2020-04-06 18:16 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
msuchanek
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <5aeaa33383e833f6eca30893fbd188b88e019eaa.1586196948.git.christophe.leroy@c-s.fr>
syscall_64.c will be reused almost as is for PPC32.
Rename it syscall.c
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/kernel/Makefile | 2 +-
arch/powerpc/kernel/{syscall_64.c => syscall.c} | 0
2 files changed, 1 insertion(+), 1 deletion(-)
rename arch/powerpc/kernel/{syscall_64.c => syscall.c} (100%)
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 570660efbb3d..8cc3c831dccd 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -49,7 +49,7 @@ obj-y := cputable.o syscalls.o \
obj-y += ptrace/
obj-$(CONFIG_PPC64) += setup_64.o sys_ppc32.o signal_64.o \
paca.o nvram_64.o firmware.o note.o \
- syscall_64.o
+ syscall.o
obj-$(CONFIG_VDSO32) += vdso32/
obj-$(CONFIG_PPC_WATCHDOG) += watchdog.o
obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o
diff --git a/arch/powerpc/kernel/syscall_64.c b/arch/powerpc/kernel/syscall.c
similarity index 100%
rename from arch/powerpc/kernel/syscall_64.c
rename to arch/powerpc/kernel/syscall.c
--
2.25.0
^ permalink raw reply related
* [RFC PATCH v3 09/15] powerpc/syscall: Make syscall_64.c buildable on PPC32
From: Christophe Leroy @ 2020-04-06 18:16 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
msuchanek
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <5aeaa33383e833f6eca30893fbd188b88e019eaa.1586196948.git.christophe.leroy@c-s.fr>
ifdef out specific PPC64 stuff to allow building
syscall_64.c on PPC32.
Modify Makefile to always build syscall.o
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/kernel/Makefile | 5 ++---
arch/powerpc/kernel/syscall.c | 9 +++++----
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 8cc3c831dccd..e4be425b7718 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -45,11 +45,10 @@ obj-y := cputable.o syscalls.o \
signal.o sysfs.o cacheinfo.o time.o \
prom.o traps.o setup-common.o \
udbg.o misc.o io.o misc_$(BITS).o \
- of_platform.o prom_parse.o
+ of_platform.o prom_parse.o syscall.o
obj-y += ptrace/
obj-$(CONFIG_PPC64) += setup_64.o sys_ppc32.o signal_64.o \
- paca.o nvram_64.o firmware.o note.o \
- syscall.o
+ paca.o nvram_64.o firmware.o note.o
obj-$(CONFIG_VDSO32) += vdso32/
obj-$(CONFIG_PPC_WATCHDOG) += watchdog.o
obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o
diff --git a/arch/powerpc/kernel/syscall.c b/arch/powerpc/kernel/syscall.c
index 4c46f3aefaf8..98c98ce12f7d 100644
--- a/arch/powerpc/kernel/syscall.c
+++ b/arch/powerpc/kernel/syscall.c
@@ -34,7 +34,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(IS_ENABLED(CONFIG_PPC64) && get_softe(regs) != IRQS_ENABLED);
account_cpu_user_entry();
@@ -56,7 +56,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;
+ set_softe(regs, IRQS_ENABLED);
local_irq_enable();
@@ -114,6 +114,7 @@ static notrace inline bool prep_irq_for_enabled_exit(void)
/* This pattern matches prep_irq_for_idle */
__hard_EE_RI_disable();
+#ifdef CONFIG_PPC64
if (unlikely(lazy_irq_pending())) {
/* Took an interrupt, may have more exit work to do. */
__hard_RI_enable();
@@ -124,7 +125,7 @@ static notrace inline bool prep_irq_for_enabled_exit(void)
}
local_paca->irq_happened = 0;
irq_soft_mask_set(IRQS_ENABLED);
-
+#endif
return true;
}
@@ -227,7 +228,7 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
return ret;
}
-#ifdef CONFIG_PPC_BOOK3S /* BOOK3E not yet using this */
+#ifdef CONFIG_PPC_BOOK3S_64 /* BOOK3E not yet using this */
notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned long msr)
{
#ifdef CONFIG_PPC_BOOK3E
--
2.25.0
^ permalink raw reply related
* [RFC PATCH v3 07/15] powerpc/irq: Add stub irq_soft_mask_return() for PPC32
From: Christophe Leroy @ 2020-04-06 18:16 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
msuchanek
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <5aeaa33383e833f6eca30893fbd188b88e019eaa.1586196948.git.christophe.leroy@c-s.fr>
To allow building syscall_64.c smoothly on PPC32, add stub version
of irq_soft_mask_return().
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
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 8c30a72262fd..1c25a84a3159 100644
--- a/arch/powerpc/include/asm/hw_irq.h
+++ b/arch/powerpc/include/asm/hw_irq.h
@@ -293,6 +293,11 @@ static inline void set_softe(struct pt_regs *regs, unsigned long val)
}
#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 related
* [RFC PATCH v3 10/15] powerpc/syscall: Use is_compat_task()
From: Christophe Leroy @ 2020-04-06 18:16 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
msuchanek
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <5aeaa33383e833f6eca30893fbd188b88e019eaa.1586196948.git.christophe.leroy@c-s.fr>
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@c-s.fr>
---
arch/powerpc/kernel/syscall.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/syscall.c b/arch/powerpc/kernel/syscall.c
index 98c98ce12f7d..0ad4250d2ce8 100644
--- a/arch/powerpc/kernel/syscall.c
+++ b/arch/powerpc/kernel/syscall.c
@@ -1,6 +1,8 @@
// SPDX-License-Identifier: GPL-2.0-or-later
#include <linux/err.h>
+#include <linux/compat.h>
+
#include <asm/asm-prototypes.h>
#include <asm/kup.h>
#include <asm/cputime.h>
@@ -86,7 +88,7 @@ notrace long system_call_exception(long r3, long r4, long r5,
/* May be faster to do array_index_nospec? */
barrier_nospec();
- if (unlikely(ti_flags & _TIF_32BIT)) {
+ if (is_compat_task()) {
f = (void *)compat_sys_call_table[r0];
r3 &= 0x00000000ffffffffULL;
--
2.25.0
^ permalink raw reply related
* [RFC PATCH v3 11/15] powerpc/syscall: Save r3 in regs->orig_r3
From: Christophe Leroy @ 2020-04-06 18:16 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
msuchanek
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <5aeaa33383e833f6eca30893fbd188b88e019eaa.1586196948.git.christophe.leroy@c-s.fr>
Save r3 in regs->orig_r3 in system_call_exception()
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
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 63f0a4414618..5ccb65f75712 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -114,7 +114,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/syscall.c b/arch/powerpc/kernel/syscall.c
index 0ad4250d2ce8..dfd7b28239b8 100644
--- a/arch/powerpc/kernel/syscall.c
+++ b/arch/powerpc/kernel/syscall.c
@@ -27,6 +27,8 @@ notrace long system_call_exception(long r3, long r4, long r5,
unsigned long ti_flags;
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 related
* [RFC PATCH v3 12/15] powerpc/syscall: Selectively check MSR_RI and MSR_PR on syscall entry
From: Christophe Leroy @ 2020-04-06 18:16 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
msuchanek
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <5aeaa33383e833f6eca30893fbd188b88e019eaa.1586196948.git.christophe.leroy@c-s.fr>
In system_call_exception(), MSR_RI needs to also be checked on 8xx.
Only book3e doesn't have MSR_RI.
On PPC32, MSR_PR is checked in real mode to avoid clobbering the
stack, so no need to check and panic in system_call_exception().
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/kernel/syscall.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/syscall.c b/arch/powerpc/kernel/syscall.c
index dfd7b28239b8..f9fca9985b0f 100644
--- a/arch/powerpc/kernel/syscall.c
+++ b/arch/powerpc/kernel/syscall.c
@@ -34,9 +34,9 @@ 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_PPC_BOOK3E))
BUG_ON(!(regs->msr & MSR_RI));
- BUG_ON(!(regs->msr & MSR_PR));
+ BUG_ON(IS_ENABLED(CONFIG_PPC64) && !(regs->msr & MSR_PR));
BUG_ON(!FULL_REGS(regs));
BUG_ON(IS_ENABLED(CONFIG_PPC64) && get_softe(regs) != IRQS_ENABLED);
--
2.25.0
^ permalink raw reply related
* [RFC PATCH v3 13/15] powerpc/syscall: system call implement entry/exit logic in C for PPC32
From: Christophe Leroy @ 2020-04-06 18:16 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
msuchanek
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <5aeaa33383e833f6eca30893fbd188b88e019eaa.1586196948.git.christophe.leroy@c-s.fr>
That's port of PPC64 syscall entry/exit logic in C to PPC32.
Performancewise:
Before : 311 cycles on null_syscall
After : 353 cycles on null_syscall
Note: before the patch, if calling NVGPRS all the time as well,
we have 335 cycles on null_syscall
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/kernel/entry_32.S | 259 ++++-----------------------------
arch/powerpc/kernel/head_32.h | 16 +-
2 files changed, 29 insertions(+), 246 deletions(-)
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index a6371fb8f761..103f5158bc44 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -315,162 +315,37 @@ stack_ovf:
RFI
#endif
-#ifdef CONFIG_TRACE_IRQFLAGS
-trace_syscall_entry_irq_off:
- /*
- * Syscall shouldn't happen while interrupts are disabled,
- * so let's do a warning here.
- */
-0: trap
- EMIT_BUG_ENTRY 0b,__FILE__,__LINE__, BUGFLAG_WARNING
- bl trace_hardirqs_on
-
- /* Now enable for real */
- LOAD_REG_IMMEDIATE(r10, MSR_KERNEL | MSR_EE)
- mtmsr r10
-
- REST_GPR(0, r1)
- REST_4GPRS(3, r1)
- REST_2GPRS(7, r1)
- b DoSyscall
-#endif /* CONFIG_TRACE_IRQFLAGS */
-
.globl transfer_to_syscall
transfer_to_syscall:
-#ifdef CONFIG_TRACE_IRQFLAGS
- andi. r12,r9,MSR_EE
- beq- trace_syscall_entry_irq_off
-#endif /* CONFIG_TRACE_IRQFLAGS */
-
-/*
- * Handle a system call.
- */
- .stabs "arch/powerpc/kernel/",N_SO,0,0,0f
- .stabs "entry_32.S",N_SO,0,0,0f
-0:
-
-_GLOBAL(DoSyscall)
- stw r3,ORIG_GPR3(r1)
- li r12,0
- stw r12,RESULT(r1)
-#ifdef CONFIG_TRACE_IRQFLAGS
- /* Make sure interrupts are enabled */
- mfmsr r11
- andi. r12,r11,MSR_EE
- /* We came in with interrupts disabled, we WARN and mark them enabled
- * for lockdep now */
-0: tweqi r12, 0
- EMIT_BUG_ENTRY 0b,__FILE__,__LINE__, BUGFLAG_WARNING
-#endif /* CONFIG_TRACE_IRQFLAGS */
- lwz r11,TI_FLAGS(r2)
- andi. r11,r11,_TIF_SYSCALL_DOTRACE
- bne- syscall_dotrace
-syscall_dotrace_cont:
- cmplwi 0,r0,NR_syscalls
- lis r10,sys_call_table@h
- ori r10,r10,sys_call_table@l
- slwi r0,r0,2
- bge- 66f
-
- barrier_nospec_asm
- /*
- * Prevent the load of the handler below (based on the user-passed
- * system call number) being speculatively executed until the test
- * against NR_syscalls and branch to .66f above has
- * committed.
- */
-
- lwzx r10,r10,r0 /* Fetch system call handler [ptr] */
- mtlr r10
- addi r9,r1,STACK_FRAME_OVERHEAD
- PPC440EP_ERR42
- blrl /* Call handler */
- .globl ret_from_syscall
+ mr r9, r0
+ addi r10, r1, STACK_FRAME_OVERHEAD
+ bl system_call_exception
ret_from_syscall:
-#ifdef CONFIG_DEBUG_RSEQ
- /* Check whether the syscall is issued inside a restartable sequence */
- stw r3,GPR3(r1)
- addi r3,r1,STACK_FRAME_OVERHEAD
- bl rseq_syscall
- lwz r3,GPR3(r1)
-#endif
- mr r6,r3
- /* disable interrupts so current_thread_info()->flags can't change */
- LOAD_REG_IMMEDIATE(r10,MSR_KERNEL) /* doesn't include MSR_EE */
- /* Note: We don't bother telling lockdep about it */
- SYNC
- mtmsr r10
- lwz r9,TI_FLAGS(r2)
- li r8,-MAX_ERRNO
- andi. r0,r9,(_TIF_SYSCALL_DOTRACE|_TIF_SINGLESTEP|_TIF_USER_WORK_MASK|_TIF_PERSYSCALL_MASK)
- bne- syscall_exit_work
- cmplw 0,r3,r8
- blt+ syscall_exit_cont
- lwz r11,_CCR(r1) /* Load CR */
- neg r3,r3
- oris r11,r11,0x1000 /* Set SO bit in CR */
- stw r11,_CCR(r1)
-syscall_exit_cont:
- lwz r8,_MSR(r1)
-#ifdef CONFIG_TRACE_IRQFLAGS
- /* If we are going to return from the syscall with interrupts
- * off, we trace that here. It shouldn't normally happen.
- */
- andi. r10,r8,MSR_EE
- bne+ 1f
- stw r3,GPR3(r1)
- bl trace_hardirqs_off
- lwz r3,GPR3(r1)
-1:
-#endif /* CONFIG_TRACE_IRQFLAGS */
-#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_44x
-BEGIN_MMU_FTR_SECTION
- lis r4,icache_44x_need_flush@ha
- lwz r5,icache_44x_need_flush@l(r4)
- cmplwi cr0,r5,0
- bne- 2f
-1:
-END_MMU_FTR_SECTION_IFCLR(MMU_FTR_TYPE_47x)
-#endif /* CONFIG_44x */
-BEGIN_FTR_SECTION
- lwarx r7,0,r1
-END_FTR_SECTION_IFSET(CPU_FTR_NEED_PAIRED_STWCX)
- stwcx. r0,0,r1 /* to clear the reservation */
- ACCOUNT_CPU_USER_EXIT(r2, r5, r7)
-#ifdef CONFIG_PPC_BOOK3S_32
- kuep_unlock r5, r7
-#endif
- kuap_check r2, r4
- lwz r4,_LINK(r1)
- lwz r5,_CCR(r1)
- mtlr r4
- mtcr r5
- lwz r7,_NIP(r1)
- lwz r2,GPR2(r1)
- lwz r1,GPR1(r1)
-#if defined(CONFIG_PPC_8xx) && defined(CONFIG_PERF_EVENTS)
- mtspr SPRN_NRI, r0
-#endif
- mtspr SPRN_SRR0,r7
- mtspr SPRN_SRR1,r8
- SYNC
- RFI
-#ifdef CONFIG_44x
-2: li r7,0
- iccci r0,r0
- stw r7,icache_44x_need_flush@l(r4)
+ addi r4, r1, STACK_FRAME_OVERHEAD
+ bl syscall_exit_prepare
+ lwz r2, _CCR(r1)
+ lwz r4, _NIP(r1)
+ lwz r5, _MSR(r1)
+ lwz r6, _LINK(r1)
+ mtspr SPRN_SRR0, r4
+ mtspr SPRN_SRR1, r5
+ mtlr r6
+ cmpwi r3, 0
+ bne 2f
+1: mtcr r2
+ REST_GPR(2, r1)
+ REST_GPR(3, r1)
+ REST_GPR(1, r1)
+ rfi
+2: lwz r3, _CTR(r1)
+ lwz r4, _XER(r1)
+ REST_NVGPRS(r1)
+ mtctr r3
+ mtspr SPRN_XER, r4
+ REST_GPR(0, r1)
+ REST_8GPRS(4, r1)
+ REST_GPR(12, r1)
b 1b
-#endif /* CONFIG_44x */
-
-66: li r3,-ENOSYS
- b ret_from_syscall
.globl ret_from_fork
ret_from_fork:
@@ -490,86 +365,6 @@ ret_from_kernel_thread:
li r3,0
b ret_from_syscall
-/* Traced system call support */
-syscall_dotrace:
- SAVE_NVGPRS(r1)
- li r0,0xc00
- stw r0,_TRAP(r1)
- addi r3,r1,STACK_FRAME_OVERHEAD
- bl do_syscall_trace_enter
- /*
- * Restore argument registers possibly just changed.
- * We use the return value of do_syscall_trace_enter
- * for call number to look up in the table (r0).
- */
- mr r0,r3
- lwz r3,GPR3(r1)
- lwz r4,GPR4(r1)
- lwz r5,GPR5(r1)
- lwz r6,GPR6(r1)
- lwz r7,GPR7(r1)
- lwz r8,GPR8(r1)
- REST_NVGPRS(r1)
-
- cmplwi r0,NR_syscalls
- /* Return code is already in r3 thanks to do_syscall_trace_enter() */
- bge- ret_from_syscall
- b syscall_dotrace_cont
-
-syscall_exit_work:
- andi. r0,r9,_TIF_RESTOREALL
- beq+ 0f
- REST_NVGPRS(r1)
- b 2f
-0: cmplw 0,r3,r8
- blt+ 1f
- andi. r0,r9,_TIF_NOERROR
- bne- 1f
- lwz r11,_CCR(r1) /* Load CR */
- neg r3,r3
- oris r11,r11,0x1000 /* Set SO bit in CR */
- stw r11,_CCR(r1)
-
-1: stw r6,RESULT(r1) /* Save result */
- stw r3,GPR3(r1) /* Update return value */
-2: andi. r0,r9,(_TIF_PERSYSCALL_MASK)
- beq 4f
-
- /* Clear per-syscall TIF flags if any are set. */
-
- li r11,_TIF_PERSYSCALL_MASK
- addi r12,r2,TI_FLAGS
-3: lwarx r8,0,r12
- andc r8,r8,r11
-#ifdef CONFIG_IBM405_ERR77
- dcbt 0,r12
-#endif
- stwcx. r8,0,r12
- bne- 3b
-
-4: /* Anything which requires enabling interrupts? */
- andi. r0,r9,(_TIF_SYSCALL_DOTRACE|_TIF_SINGLESTEP)
- beq ret_from_except
-
- /* Re-enable interrupts. There is no need to trace that with
- * lockdep as we are supposed to have IRQs on at this point
- */
- ori r10,r10,MSR_EE
- SYNC
- mtmsr r10
-
- /* Save NVGPRS if they're not saved already */
- lwz r4,_TRAP(r1)
- andi. r4,r4,1
- beq 5f
- SAVE_NVGPRS(r1)
- li r4,0xc00
- stw r4,_TRAP(r1)
-5:
- addi r3,r1,STACK_FRAME_OVERHEAD
- bl do_syscall_trace_leave
- b ret_from_except_full
-
/*
* System call was called from kernel. We get here with SRR1 in r9.
* Mark the exception as recoverable once we have retrieved SRR0,
diff --git a/arch/powerpc/kernel/head_32.h b/arch/powerpc/kernel/head_32.h
index 9abec6cd099c..c301d666a3e5 100644
--- a/arch/powerpc/kernel/head_32.h
+++ b/arch/powerpc/kernel/head_32.h
@@ -174,12 +174,13 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_HPTE_TABLE)
stw r2,GPR2(r11)
addi r10,r10,STACK_FRAME_REGS_MARKER@l
stw r9,_MSR(r11)
- li r2, \trapno + 1
+ li r2, \trapno
stw r10,8(r11)
stw r2,_TRAP(r11)
SAVE_GPR(0, r11)
SAVE_4GPRS(3, r11)
SAVE_2GPRS(7, r11)
+ SAVE_NVGPRS(r11)
addi r11,r1,STACK_FRAME_OVERHEAD
addi r2,r12,-THREAD
stw r11,PT_REGS(r12)
@@ -188,9 +189,6 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_HPTE_TABLE)
internal debug mode bit to do this. */
lwz r12,THREAD_DBCR0(r12)
andis. r12,r12,DBCR0_IDM@h
-#endif
- ACCOUNT_CPU_USER_ENTRY(r2, r11, r12)
-#if defined(CONFIG_40x)
beq+ 3f
/* From user and task is ptraced - load up global dbcr0 */
li r12,-1 /* clear all pending debug events */
@@ -209,17 +207,7 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_HPTE_TABLE)
tovirt_novmstack r2, r2 /* set r2 to current */
lis r11, transfer_to_syscall@h
ori r11, r11, transfer_to_syscall@l
-#ifdef CONFIG_TRACE_IRQFLAGS
- /*
- * If MSR is changing we need to keep interrupts disabled at this point
- * otherwise we might risk taking an interrupt before we tell lockdep
- * they are enabled.
- */
LOAD_REG_IMMEDIATE(r10, MSR_KERNEL)
- rlwimi r10, r9, 0, MSR_EE
-#else
- LOAD_REG_IMMEDIATE(r10, MSR_KERNEL | MSR_EE)
-#endif
#if defined(CONFIG_PPC_8xx) && defined(CONFIG_PERF_EVENTS)
mtspr SPRN_NRI, r0
#endif
--
2.25.0
^ permalink raw reply related
* [RFC PATCH v3 14/15] powerpc/syscall: Avoid stack frame in likely part of syscall_call_exception()
From: Christophe Leroy @ 2020-04-06 18:16 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
msuchanek
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <5aeaa33383e833f6eca30893fbd188b88e019eaa.1586196948.git.christophe.leroy@c-s.fr>
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 syscall_call_exception()
Before : 353 cycles on null_syscall
After : 347 cycles on null_syscall
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@c-s.fr>
---
arch/powerpc/kernel/syscall.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/powerpc/kernel/syscall.c b/arch/powerpc/kernel/syscall.c
index f9fca9985b0f..af449a4a8e8f 100644
--- a/arch/powerpc/kernel/syscall.c
+++ b/arch/powerpc/kernel/syscall.c
@@ -85,6 +85,9 @@ notrace long system_call_exception(long r3, long r4, long r5,
} else if (unlikely(r0 >= NR_syscalls)) {
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
* [RFC PATCH v3 15/15] powerpc/kernel: Do not inconditionally save non volatile registers on system call
From: Christophe Leroy @ 2020-04-06 18:16 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
msuchanek
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <5aeaa33383e833f6eca30893fbd188b88e019eaa.1586196948.git.christophe.leroy@c-s.fr>
To allow that, syscall_exit_prepare() gets split in 3 parts.
On PPC32, the three parts are called from entry_32.S
On PPC64, we keep a syscall_exit_prepare() function which
concatenates the three parts.
One benefit is also that the likely part of
syscall_exit_prepare_begin() and the syscall_exit_prepare_end()
functions are frameless whereas there was no way to get the
likely part of syscall_exit_prepare() frameless.
Before : 347 cycles on null_syscall
After : 307 cycles on null_syscall, ie better than before C porting.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/include/asm/asm-prototypes.h | 11 +++
arch/powerpc/kernel/entry_32.S | 25 ++++++-
arch/powerpc/kernel/head_32.h | 3 +-
arch/powerpc/kernel/syscall.c | 83 +++++++++++++++--------
4 files changed, 92 insertions(+), 30 deletions(-)
diff --git a/arch/powerpc/include/asm/asm-prototypes.h b/arch/powerpc/include/asm/asm-prototypes.h
index 7d81e86a1e5d..eea5133733bb 100644
--- a/arch/powerpc/include/asm/asm-prototypes.h
+++ b/arch/powerpc/include/asm/asm-prototypes.h
@@ -98,6 +98,17 @@ unsigned long __init early_init(unsigned long dt_ptr);
void __init machine_init(u64 dt_ptr);
#endif
long system_call_exception(long r3, long r4, long r5, long r6, long r7, long r8, unsigned long r0, struct pt_regs *regs);
+#ifdef CONFIG_PPC64
+#define static64 static
+#else
+#define static64
+#endif
+static64 notrace unsigned long
+syscall_exit_prepare_begin(unsigned long r3, struct pt_regs *regs, unsigned long ti_flags);
+static64 notrace unsigned long
+syscall_exit_prepare_loop(unsigned long ret, struct pt_regs *regs, unsigned long ti_flags);
+static64 notrace unsigned long
+syscall_exit_prepare_end(unsigned long ret, struct pt_regs *regs, unsigned long ti_flags);
notrace unsigned long syscall_exit_prepare(unsigned long r3, struct pt_regs *regs);
notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned long msr);
notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsigned long msr);
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 103f5158bc44..b9287fd0fcc6 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -315,14 +315,37 @@ stack_ovf:
RFI
#endif
+save_nvgprs:
+ lwz r11, _TRAP(r1)
+ andi. r12, r11, 1
+ rlwinm r11, r11, 0, ~1
+ beqlr
+ SAVE_NVGPRS(r1)
+ stw r11, _TRAP(r1)
+ blr
+
.globl transfer_to_syscall
transfer_to_syscall:
+ lwz r10, TI_FLAGS(r2)
mr r9, r0
+ andi. r10, r10, _TIF_SYSCALL_DOTRACE
addi r10, r1, STACK_FRAME_OVERHEAD
+ bnel- save_nvgprs
bl system_call_exception
ret_from_syscall:
+ lwz r5, TI_FLAGS(r2)
addi r4, r1, STACK_FRAME_OVERHEAD
- bl syscall_exit_prepare
+ andi. r0, r5, _TIF_SYSCALL_DOTRACE | _TIF_SINGLESTEP | _TIF_USER_WORK_MASK
+ bnel- save_nvgprs
+ bl syscall_exit_prepare_begin
+1: lwz r5, TI_FLAGS(r2)
+ addi r4, r1, STACK_FRAME_OVERHEAD
+ andi. r0, r5, _TIF_USER_WORK_MASK & ~_TIF_RESTORE_TM
+ beq+ 1f
+ bl save_nvgprs
+ bl syscall_exit_prepare_loop
+ b 1b
+1: bl syscall_exit_prepare_end
lwz r2, _CCR(r1)
lwz r4, _NIP(r1)
lwz r5, _MSR(r1)
diff --git a/arch/powerpc/kernel/head_32.h b/arch/powerpc/kernel/head_32.h
index c301d666a3e5..1cc9a67cb42c 100644
--- a/arch/powerpc/kernel/head_32.h
+++ b/arch/powerpc/kernel/head_32.h
@@ -174,13 +174,12 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_HPTE_TABLE)
stw r2,GPR2(r11)
addi r10,r10,STACK_FRAME_REGS_MARKER@l
stw r9,_MSR(r11)
- li r2, \trapno
+ li r2, \trapno + 1
stw r10,8(r11)
stw r2,_TRAP(r11)
SAVE_GPR(0, r11)
SAVE_4GPRS(3, r11)
SAVE_2GPRS(7, r11)
- SAVE_NVGPRS(r11)
addi r11,r1,STACK_FRAME_OVERHEAD
addi r2,r12,-THREAD
stw r11,PT_REGS(r12)
diff --git a/arch/powerpc/kernel/syscall.c b/arch/powerpc/kernel/syscall.c
index af449a4a8e8f..b15f19c00ccb 100644
--- a/arch/powerpc/kernel/syscall.c
+++ b/arch/powerpc/kernel/syscall.c
@@ -37,7 +37,7 @@ notrace long system_call_exception(long r3, long r4, long r5,
if (!IS_ENABLED(CONFIG_PPC_BOOK3E))
BUG_ON(!(regs->msr & MSR_RI));
BUG_ON(IS_ENABLED(CONFIG_PPC64) && !(regs->msr & MSR_PR));
- BUG_ON(!FULL_REGS(regs));
+ BUG_ON(IS_ENABLED(CONFIG_PPC64) && !FULL_REGS(regs));
BUG_ON(IS_ENABLED(CONFIG_PPC64) && get_softe(regs) != IRQS_ENABLED);
account_cpu_user_entry();
@@ -145,11 +145,9 @@ static notrace inline bool prep_irq_for_enabled_exit(void)
* The function graph tracer can not trace the return side of this function,
* because RI=0 and soft mask state is "unreconciled", so it is marked notrace.
*/
-notrace unsigned long syscall_exit_prepare(unsigned long r3,
- struct pt_regs *regs)
+static64 notrace unsigned long
+syscall_exit_prepare_begin(unsigned long r3, struct pt_regs *regs, unsigned long ti_flags)
{
- unsigned long *ti_flagsp = ¤t_thread_info()->flags;
- unsigned long ti_flags;
unsigned long ret = 0;
regs->result = r3;
@@ -157,8 +155,6 @@ 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;
-
if (unlikely(r3 >= (unsigned long)-MAX_ERRNO)) {
if (likely(!(ti_flags & (_TIF_NOERROR | _TIF_RESTOREALL)))) {
r3 = -r3;
@@ -171,7 +167,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;
}
@@ -181,27 +177,35 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
ret |= _TIF_RESTOREALL;
}
-again:
local_irq_disable();
- ti_flags = READ_ONCE(*ti_flagsp);
- while (unlikely(ti_flags & (_TIF_USER_WORK_MASK & ~_TIF_RESTORE_TM))) {
- local_irq_enable();
- if (ti_flags & _TIF_NEED_RESCHED) {
- schedule();
- } else {
- /*
- * SIGPENDING must restore signal handler function
- * argument GPRs, and some non-volatiles (e.g., r1).
- * Restore all for now. This could be made lighter.
- */
- if (ti_flags & _TIF_SIGPENDING)
- ret |= _TIF_RESTOREALL;
- do_notify_resume(regs, ti_flags);
- }
- local_irq_disable();
- ti_flags = READ_ONCE(*ti_flagsp);
+
+ return ret;
+}
+
+static64 notrace unsigned long
+syscall_exit_prepare_loop(unsigned long ret, struct pt_regs *regs, unsigned long ti_flags)
+{
+ local_irq_enable();
+ if (ti_flags & _TIF_NEED_RESCHED) {
+ schedule();
+ } else {
+ /*
+ * SIGPENDING must restore signal handler function
+ * argument GPRs, and some non-volatiles (e.g., r1).
+ * Restore all for now. This could be made lighter.
+ */
+ if (ti_flags & _TIF_SIGPENDING)
+ ret |= _TIF_RESTOREALL;
+ do_notify_resume(regs, ti_flags);
}
+ local_irq_disable();
+
+ return ret;
+}
+static64 notrace unsigned long
+syscall_exit_prepare_end(unsigned long ret, struct pt_regs *regs, unsigned long ti_flags)
+{
if (IS_ENABLED(CONFIG_PPC_BOOK3S) && IS_ENABLED(CONFIG_PPC_FPU)) {
if (IS_ENABLED(CONFIG_PPC_TRANSACTIONAL_MEM) &&
unlikely((ti_flags & _TIF_RESTORE_TM))) {
@@ -221,7 +225,8 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
if (unlikely(!prep_irq_for_enabled_exit())) {
local_irq_enable();
- goto again;
+ local_irq_disable();
+ return ret | 0x80000000;
}
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
@@ -235,6 +240,30 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
return ret;
}
+#ifdef CONFIG_PPC64
+notrace unsigned long syscall_exit_prepare(unsigned long r3, struct pt_regs *regs)
+{
+ unsigned long ret;
+ unsigned long *ti_flagsp = ¤t_thread_info()->flags;
+ unsigned long ti_flags = *ti_flagsp;
+
+ ret = syscall_exit_prepare_begin(r3, regs, ti_flags);
+
+again:
+ ti_flags = READ_ONCE(*ti_flagsp);
+ if (unlikely(ti_flags & (_TIF_USER_WORK_MASK & ~_TIF_RESTORE_TM))) {
+ ret = syscall_exit_prepare_loop(ret, regs, ti_flags);
+ goto again;
+ }
+ ret = syscall_exit_prepare_end(ret, regs, ti_flags);
+ if (unlikely(ret & 0x80000000)) {
+ ret &= ~0x80000000;
+ goto again;
+ }
+ return ret;
+}
+#endif
+
#ifdef CONFIG_PPC_BOOK3S_64 /* BOOK3E not yet using this */
notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned long msr)
{
--
2.25.0
^ permalink raw reply related
* Re: [RFC PATCH v2 12/13] powerpc/kernel: Do not inconditionally save non volatile registers on system call
From: Christophe Leroy @ 2020-04-06 18:18 UTC (permalink / raw)
To: Nicholas Piggin, Benjamin Herrenschmidt, Michael Ellerman,
msuchanek, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1586135554.pnqaj0giue.astroid@bobo.none>
Le 06/04/2020 à 03:25, Nicholas Piggin a écrit :
> Christophe Leroy's on April 6, 2020 3:44 am:
>> Before : 347 cycles on null_syscall
>> After : 327 cycles on null_syscall
>
> The problem I had doing this is that signal delivery wnats full regs,
> and you don't know if you have a signal pending ahead of time if you
> have interrupts enabled.
>
> I began to try bailing out back to asm to save nvgprs and call again.
> I think that can be made to work, but it is more complication in asm,
> and I soon found that 64s CPUs don't care about NVGPRs too much so it's
> nice to get rid of the !fullregs state.
I tried a new way in v3, please have a look. I split
syscall_exit_prepare() in 3 parts and the result is unexpected: it is
better than before the series (307 cycles now versus 311 cycles with
full ASM syscall entry/exit).
>
> Possibly another approach would be to leave interrupts disabled for the
> case where you have no work to do. You could create a small
> syscall_exit_prepare_nowork fastpath for that case for 32-bit, perhaps?
>
> Thanks,
> Nick
>
Christophe
^ permalink raw reply
* Re: [RFC PATCH v2 11/13] powerpc/syscall: Avoid stack frame in likely part of syscall_call_exception()
From: Christophe Leroy @ 2020-04-06 18:19 UTC (permalink / raw)
To: Nicholas Piggin, Benjamin Herrenschmidt, Michael Ellerman,
msuchanek, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1586136357.atgut3zasc.astroid@bobo.none>
Le 06/04/2020 à 03:29, Nicholas Piggin a écrit :
> Christophe Leroy's on April 6, 2020 3:44 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 syscall_call_exception()
>>
>> Before : 353 cycles on null_syscall
>> After : 347 cycles on null_syscall
>>
[...]
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> ---
>> arch/powerpc/kernel/syscall.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/powerpc/kernel/syscall.c b/arch/powerpc/kernel/syscall.c
>> index 69d75fc4a5eb..630c423e089a 100644
>> --- a/arch/powerpc/kernel/syscall.c
>> +++ b/arch/powerpc/kernel/syscall.c
>> @@ -91,6 +91,8 @@ notrace long system_call_exception(long r3, long r4, long r5,
>>
>> } else if (unlikely(r0 >= NR_syscalls)) {
>> return -ENOSYS;
>> + } else {
>> + r3 = regs->orig_gpr3;
>> }
>
> So this just gives enough volatiles to avoid spilling to stack? I wonder
> about other various options here if they would cause a spill anyway.
>
> Interesting optimisation, it would definitely need a comment. Would be
> nice if we had a way to tell the compiler that a local can be reloaded
> from a particular address.
Ok, comment added.
Christophe
^ permalink raw reply
* Re: [RFC PATCH v2 05/13] powerpc/syscall: Rename syscall_64.c into syscall.c
From: Christophe Leroy @ 2020-04-06 18:20 UTC (permalink / raw)
To: Nicholas Piggin, Benjamin Herrenschmidt, Michael Ellerman,
msuchanek, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1586137301.c2ssus5vmb.astroid@bobo.none>
Le 06/04/2020 à 03:42, Nicholas Piggin a écrit :
> Christophe Leroy's on April 6, 2020 3:44 am:
>> syscall_64.c will be reused almost as is for PPC32.
>>
>> Rename it syscall.c
>
> Don't mind this, but I wonder if we can rename it to interrupt.c.
Interrupt for me is irq.
Maybe exception.c ?
Exceptions, that's what interrupts and system calls are.
Christophe
^ permalink raw reply
* Re: [RFC PATCH v2 06/13] powerpc/syscall: Make syscall_64.c buildable on PPC32
From: Christophe Leroy @ 2020-04-06 18:21 UTC (permalink / raw)
To: Nicholas Piggin, Benjamin Herrenschmidt, Michael Ellerman,
msuchanek, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1586137334.pcovkdryot.astroid@bobo.none>
Le 06/04/2020 à 03:52, Nicholas Piggin a écrit :
> Christophe Leroy's on April 6, 2020 3:44 am:
>> ifdef out specific PPC64 stuff to allow building
>> syscall_64.c on PPC32.
>>
>> Modify Makefile to always build syscall.o
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> ---
>> arch/powerpc/kernel/Makefile | 5 ++---
>> arch/powerpc/kernel/syscall.c | 10 ++++++++++
>> 2 files changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
>> index 8cc3c831dccd..e4be425b7718 100644
>> --- a/arch/powerpc/kernel/Makefile
>> +++ b/arch/powerpc/kernel/Makefile
>> @@ -45,11 +45,10 @@ obj-y := cputable.o syscalls.o \
>> signal.o sysfs.o cacheinfo.o time.o \
>> prom.o traps.o setup-common.o \
>> udbg.o misc.o io.o misc_$(BITS).o \
>> - of_platform.o prom_parse.o
>> + of_platform.o prom_parse.o syscall.o
>> obj-y += ptrace/
>> obj-$(CONFIG_PPC64) += setup_64.o sys_ppc32.o signal_64.o \
>> - paca.o nvram_64.o firmware.o note.o \
>> - syscall.o
>> + paca.o nvram_64.o firmware.o note.o
>> obj-$(CONFIG_VDSO32) += vdso32/
>> obj-$(CONFIG_PPC_WATCHDOG) += watchdog.o
>> obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o
>> diff --git a/arch/powerpc/kernel/syscall.c b/arch/powerpc/kernel/syscall.c
>> index 72f3d2f0a823..28bd43db8755 100644
>> --- a/arch/powerpc/kernel/syscall.c
>> +++ b/arch/powerpc/kernel/syscall.c
>> @@ -25,8 +25,10 @@ notrace long system_call_exception(long r3, long r4, long r5,
>> unsigned long ti_flags;
>> syscall_fn f;
>>
>> +#ifdef CONFIG_PPC64
>> if (IS_ENABLED(CONFIG_PPC_IRQ_SOFT_MASK_DEBUG))
>> BUG_ON(irq_soft_mask_return() != IRQS_ALL_DISABLED);
>> +#endif
>>
>> trace_hardirqs_off(); /* finish reconciling */
>>
>> @@ -34,7 +36,9 @@ 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));
>> +#ifdef CONFIG_PPC64
>> BUG_ON(regs->softe != IRQS_ENABLED);
>> +#endif
>>
>> account_cpu_user_entry();
>>
>> @@ -56,7 +60,9 @@ 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!
>> */
>> +#ifdef CONFIG_PPC64
>> regs->softe = IRQS_ENABLED;
>> +#endif
>>
>> local_irq_enable();
>>
>> @@ -148,7 +154,9 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
>> ret |= _TIF_RESTOREALL;
>> }
>>
>> +#ifdef CONFIG_PPC64
>> again:
>> +#endif
>> local_irq_disable();
>> ti_flags = READ_ONCE(*ti_flagsp);
>> while (unlikely(ti_flags & (_TIF_USER_WORK_MASK & ~_TIF_RESTORE_TM))) {
>> @@ -191,6 +199,7 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
>>
>> /* This pattern matches prep_irq_for_idle */
>> __hard_EE_RI_disable();
>> +#ifdef CONFIG_PPC64
>> if (unlikely(lazy_irq_pending())) {
>> __hard_RI_enable();
>> trace_hardirqs_off();
>> @@ -201,6 +210,7 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
>> }
>> local_paca->irq_happened = 0;
>> irq_soft_mask_set(IRQS_ENABLED);
>> +#endif
>>
>> #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
>> local_paca->tm_scratch = regs->msr;
>> --
>> 2.25.0
>>
>>
>
> The #ifdefs disappoint me!
They disappoint me as well. I tried to removed most of them in v3, I
also took your patch up front the series.
>
> Here is (unested) something that should help 32-bit avoid several ifdefs
> in the main part of the function. I should have done this as part of the
> merged series, but that's okay I'll submit as a cleanup.
>
> The rest looks okay for now. Maybe we grow some helpers to manage the
> soft-mask state, though I'm not really sure it would make sense for
> 32-bit code to ever call them. Maybe just confined to this file would be
> okay but for now the ifdefs are okay.
Thanks
Christophe
^ permalink raw reply
* Re: [PATCH v3 1/1] ppc/crash: Reset spinlocks during crash
From: Leonardo Bras @ 2020-04-06 18:46 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Enrico Weigelt, Alexios Zavras, Thomas Gleixner,
Greg Kroah-Hartman, Christophe Leroy, peterz
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <871rp6t9di.fsf@mpe.ellerman.id.au>
[-- Attachment #1: Type: text/plain, Size: 545 bytes --]
On Thu, 2020-04-02 at 22:28 +1100, Michael Ellerman wrote:
> Leonardo Bras <leonardo@linux.ibm.com>
> TBH I think we could just drop that printk() entirely.
>
> Or we could tell printk() that we're in NMI context so that it uses the
> percpu buffers.
>
> We should probably do the latter anyway, in case there's any other code
> we call that inadvertently calls printk().
Done:
http://patchwork.ozlabs.org/patch/1266956/
About the rtas-call, I think it will take more time, because I have to
study it properly.
Thank you,
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v11 3/8] powerpc/perf: consolidate read_user_stack_32
From: Michal Suchánek @ 2020-04-06 20:52 UTC (permalink / raw)
To: Nicholas Piggin
Cc: Mark Rutland, Gustavo Luiz Duarte, Alexander Shishkin,
Sebastian Andrzej Siewior, linux-kernel, Paul Mackerras,
Jiri Olsa, Rob Herring, Michael Neuling, Eric Richter,
Masahiro Yamada, Nayna Jain, Peter Zijlstra, Ingo Molnar,
Hari Bathini, Jordan Niethe, Valentin Schneider, Arnd Bergmann,
Arnaldo Carvalho de Melo, Alexander Viro, Jonathan Cameron,
Namhyung Kim, Thomas Gleixner, Andy Shevchenko, Allison Randal,
Greg Kroah-Hartman, Claudio Carvalho, Mauro Carvalho Chehab,
Eric W. Biederman, linux-fsdevel, linuxppc-dev, David S. Miller,
Thiago Jung Bauermann
In-Reply-To: <1585896170.ohti800w9v.astroid@bobo.none>
On Fri, Apr 03, 2020 at 05:13:25PM +1000, Nicholas Piggin wrote:
> Michal Suchánek's on March 25, 2020 5:38 am:
> > On Tue, Mar 24, 2020 at 06:48:20PM +1000, Nicholas Piggin wrote:
> >> Michal Suchanek's on March 19, 2020 10:19 pm:
> >> > There are two almost identical copies for 32bit and 64bit.
> >> >
> >> > The function is used only in 32bit code which will be split out in next
> >> > patch so consolidate to one function.
> >> >
> >> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> >> > Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>
> >> > ---
> >> > v6: new patch
> >> > v8: move the consolidated function out of the ifdef block.
> >> > v11: rebase on top of def0bfdbd603
> >> > ---
> >> > arch/powerpc/perf/callchain.c | 48 +++++++++++++++++------------------
> >> > 1 file changed, 24 insertions(+), 24 deletions(-)
> >> >
> >> > diff --git a/arch/powerpc/perf/callchain.c b/arch/powerpc/perf/callchain.c
> >> > index cbc251981209..c9a78c6e4361 100644
> >> > --- a/arch/powerpc/perf/callchain.c
> >> > +++ b/arch/powerpc/perf/callchain.c
> >> > @@ -161,18 +161,6 @@ static int read_user_stack_64(unsigned long __user *ptr, unsigned long *ret)
> >> > return read_user_stack_slow(ptr, ret, 8);
> >> > }
> >> >
> >> > -static int read_user_stack_32(unsigned int __user *ptr, unsigned int *ret)
> >> > -{
> >> > - if ((unsigned long)ptr > TASK_SIZE - sizeof(unsigned int) ||
> >> > - ((unsigned long)ptr & 3))
> >> > - return -EFAULT;
> >> > -
> >> > - if (!probe_user_read(ret, ptr, sizeof(*ret)))
> >> > - return 0;
> >> > -
> >> > - return read_user_stack_slow(ptr, ret, 4);
> >> > -}
> >> > -
> >> > static inline int valid_user_sp(unsigned long sp, int is_64)
> >> > {
> >> > if (!sp || (sp & 7) || sp > (is_64 ? TASK_SIZE : 0x100000000UL) - 32)
> >> > @@ -277,19 +265,9 @@ static void perf_callchain_user_64(struct perf_callchain_entry_ctx *entry,
> >> > }
> >> >
> >> > #else /* CONFIG_PPC64 */
> >> > -/*
> >> > - * On 32-bit we just access the address and let hash_page create a
> >> > - * HPTE if necessary, so there is no need to fall back to reading
> >> > - * the page tables. Since this is called at interrupt level,
> >> > - * do_page_fault() won't treat a DSI as a page fault.
> >> > - */
> >> > -static int read_user_stack_32(unsigned int __user *ptr, unsigned int *ret)
> >> > +static int read_user_stack_slow(void __user *ptr, void *buf, int nb)
> >> > {
> >> > - if ((unsigned long)ptr > TASK_SIZE - sizeof(unsigned int) ||
> >> > - ((unsigned long)ptr & 3))
> >> > - return -EFAULT;
> >> > -
> >> > - return probe_user_read(ret, ptr, sizeof(*ret));
> >> > + return 0;
> >> > }
> >> >
> >> > static inline void perf_callchain_user_64(struct perf_callchain_entry_ctx *entry,
> >> > @@ -312,6 +290,28 @@ static inline int valid_user_sp(unsigned long sp, int is_64)
> >> >
> >> > #endif /* CONFIG_PPC64 */
> >> >
> >> > +/*
> >> > + * On 32-bit we just access the address and let hash_page create a
> >> > + * HPTE if necessary, so there is no need to fall back to reading
> >> > + * the page tables. Since this is called at interrupt level,
> >> > + * do_page_fault() won't treat a DSI as a page fault.
> >> > + */
> >>
> >> The comment is actually probably better to stay in the 32-bit
> >> read_user_stack_slow implementation. Is that function defined
> >> on 32-bit purely so that you can use IS_ENABLED()? In that case
> > It documents the IS_ENABLED() and that's where it is. The 32bit
> > definition is only a technical detail.
>
> Sorry for the late reply, busy trying to fix bugs in the C rewrite
> series. I don't think it is the right place, it should be in the
> ppc32 implementation detail.
Which does not exist anymore after the 32bit and 64bit part is split.
> ppc64 has an equivalent comment at the top of its read_user_stack functions.
>
> >> I would prefer to put a BUG() there which makes it self documenting.
> > Which will cause checkpatch complaints about introducing new BUG() which
> > is frowned on.
>
> It's fine in this case, that warning is about not introducing
> runtime bugs, but this wouldn't be.
>
> But... I actually don't like adding read_user_stack_slow on 32-bit
> and especially not just to make IS_ENABLED work.
That's to not break build at this point. Later the function is removed.
>
> IMO this would be better if you really want to consolidate it
>
> ---
>
> diff --git a/arch/powerpc/perf/callchain.c b/arch/powerpc/perf/callchain.c
> index cbc251981209..ca3a599b3f54 100644
> --- a/arch/powerpc/perf/callchain.c
> +++ b/arch/powerpc/perf/callchain.c
> @@ -108,7 +108,7 @@ perf_callchain_kernel(struct perf_callchain_entry_ctx *entry, struct pt_regs *re
> * interrupt context, so if the access faults, we read the page tables
> * to find which page (if any) is mapped and access it directly.
> */
> -static int read_user_stack_slow(void __user *ptr, void *buf, int nb)
> +static int read_user_stack_slow(const void __user *ptr, void *buf, int nb)
> {
> int ret = -EFAULT;
> pgd_t *pgdir;
> @@ -149,28 +149,21 @@ static int read_user_stack_slow(void __user *ptr, void *buf, int nb)
> return ret;
> }
>
> -static int read_user_stack_64(unsigned long __user *ptr, unsigned long *ret)
> +static int __read_user_stack(const void __user *ptr, void *ret, size_t size)
> {
> - if ((unsigned long)ptr > TASK_SIZE - sizeof(unsigned long) ||
> - ((unsigned long)ptr & 7))
> + if ((unsigned long)ptr > TASK_SIZE - size ||
> + ((unsigned long)ptr & (size - 1)))
> return -EFAULT;
>
> - if (!probe_user_read(ret, ptr, sizeof(*ret)))
> + if (!probe_user_read(ret, ptr, size))
> return 0;
>
> - return read_user_stack_slow(ptr, ret, 8);
> + return read_user_stack_slow(ptr, ret, size);
> }
>
> -static int read_user_stack_32(unsigned int __user *ptr, unsigned int *ret)
> +static int read_user_stack_64(unsigned long __user *ptr, unsigned long *ret)
> {
> - if ((unsigned long)ptr > TASK_SIZE - sizeof(unsigned int) ||
> - ((unsigned long)ptr & 3))
> - return -EFAULT;
> -
> - if (!probe_user_read(ret, ptr, sizeof(*ret)))
> - return 0;
> -
> - return read_user_stack_slow(ptr, ret, 4);
> + return __read_user_stack(ptr, ret, sizeof(*ret));
> }
>
> static inline int valid_user_sp(unsigned long sp, int is_64)
> @@ -283,13 +276,13 @@ static void perf_callchain_user_64(struct perf_callchain_entry_ctx *entry,
> * the page tables. Since this is called at interrupt level,
> * do_page_fault() won't treat a DSI as a page fault.
> */
> -static int read_user_stack_32(unsigned int __user *ptr, unsigned int *ret)
> +static int __read_user_stack(const void __user *ptr, void *ret, size_t size)
> {
> - if ((unsigned long)ptr > TASK_SIZE - sizeof(unsigned int) ||
> - ((unsigned long)ptr & 3))
> + if ((unsigned long)ptr > TASK_SIZE - size ||
> + ((unsigned long)ptr & (size - 1)))
> return -EFAULT;
>
> - return probe_user_read(ret, ptr, sizeof(*ret));
> + return probe_user_read(ret, ptr, size);
> }
>
> static inline void perf_callchain_user_64(struct perf_callchain_entry_ctx *entry,
> @@ -312,6 +305,11 @@ static inline int valid_user_sp(unsigned long sp, int is_64)
>
> #endif /* CONFIG_PPC64 */
>
> +static int read_user_stack_32(unsigned int __user *ptr, unsigned int *ret)
> +{
> + return __read_user_stack(ptr, ret, sizeof(*ret));
> +}
> +
> /*
> * Layout for non-RT signal frames
> */
^ permalink raw reply
* [PATCH] powerpcs: perf: consolidate perf_callchain_user_64 and perf_callchain_user_32
From: Michal Suchanek @ 2020-04-06 21:00 UTC (permalink / raw)
To: linuxppc-dev, Nicholas Piggin
Cc: Mark Rutland, Peter Zijlstra, linux-kernel,
Arnaldo Carvalho de Melo, Alexander Shishkin, Ingo Molnar,
Paul Mackerras, Namhyung Kim, Michal Suchanek, Jiri Olsa
In-Reply-To: <1585896170.ohti800w9v.astroid@bobo.none>
perf_callchain_user_64 and perf_callchain_user_32 are nearly identical.
Consolidate into one function with thin wrappers.
Suggested-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
arch/powerpc/perf/callchain.h | 24 +++++++++++++++++++++++-
arch/powerpc/perf/callchain_32.c | 21 ++-------------------
arch/powerpc/perf/callchain_64.c | 14 ++++----------
3 files changed, 29 insertions(+), 30 deletions(-)
diff --git a/arch/powerpc/perf/callchain.h b/arch/powerpc/perf/callchain.h
index 7a2cb9e1181a..7540bb71cb60 100644
--- a/arch/powerpc/perf/callchain.h
+++ b/arch/powerpc/perf/callchain.h
@@ -2,7 +2,7 @@
#ifndef _POWERPC_PERF_CALLCHAIN_H
#define _POWERPC_PERF_CALLCHAIN_H
-int read_user_stack_slow(void __user *ptr, void *buf, int nb);
+int read_user_stack_slow(const void __user *ptr, void *buf, int nb);
void perf_callchain_user_64(struct perf_callchain_entry_ctx *entry,
struct pt_regs *regs);
void perf_callchain_user_32(struct perf_callchain_entry_ctx *entry,
@@ -16,4 +16,26 @@ static inline bool invalid_user_sp(unsigned long sp)
return (!sp || (sp & mask) || (sp > top));
}
+/*
+ * On 32-bit we just access the address and let hash_page create a
+ * HPTE if necessary, so there is no need to fall back to reading
+ * the page tables. Since this is called at interrupt level,
+ * do_page_fault() won't treat a DSI as a page fault.
+ */
+static inline int __read_user_stack(const void __user *ptr, void *ret,
+ size_t size)
+{
+ int rc;
+
+ if ((unsigned long)ptr > TASK_SIZE - size ||
+ ((unsigned long)ptr & (size - 1)))
+ return -EFAULT;
+ rc = probe_user_read(ret, ptr, size);
+
+ if (rc && IS_ENABLED(CONFIG_PPC64))
+ return read_user_stack_slow(ptr, ret, size);
+
+ return rc;
+}
+
#endif /* _POWERPC_PERF_CALLCHAIN_H */
diff --git a/arch/powerpc/perf/callchain_32.c b/arch/powerpc/perf/callchain_32.c
index 8aa951003141..1b4621f177e8 100644
--- a/arch/powerpc/perf/callchain_32.c
+++ b/arch/powerpc/perf/callchain_32.c
@@ -31,26 +31,9 @@
#endif /* CONFIG_PPC64 */
-/*
- * On 32-bit we just access the address and let hash_page create a
- * HPTE if necessary, so there is no need to fall back to reading
- * the page tables. Since this is called at interrupt level,
- * do_page_fault() won't treat a DSI as a page fault.
- */
-static int read_user_stack_32(unsigned int __user *ptr, unsigned int *ret)
+static int read_user_stack_32(const unsigned int __user *ptr, unsigned int *ret)
{
- int rc;
-
- if ((unsigned long)ptr > TASK_SIZE - sizeof(unsigned int) ||
- ((unsigned long)ptr & 3))
- return -EFAULT;
-
- rc = probe_user_read(ret, ptr, sizeof(*ret));
-
- if (IS_ENABLED(CONFIG_PPC64) && rc)
- return read_user_stack_slow(ptr, ret, 4);
-
- return rc;
+ return __read_user_stack(ptr, ret, sizeof(*ret);
}
/*
diff --git a/arch/powerpc/perf/callchain_64.c b/arch/powerpc/perf/callchain_64.c
index df1ffd8b20f2..55bbc25a54ed 100644
--- a/arch/powerpc/perf/callchain_64.c
+++ b/arch/powerpc/perf/callchain_64.c
@@ -24,7 +24,7 @@
* interrupt context, so if the access faults, we read the page tables
* to find which page (if any) is mapped and access it directly.
*/
-int read_user_stack_slow(void __user *ptr, void *buf, int nb)
+int read_user_stack_slow(const void __user *ptr, void *buf, int nb)
{
int ret = -EFAULT;
pgd_t *pgdir;
@@ -65,16 +65,10 @@ int read_user_stack_slow(void __user *ptr, void *buf, int nb)
return ret;
}
-static int read_user_stack_64(unsigned long __user *ptr, unsigned long *ret)
+static int read_user_stack_64(const unsigned long __user *ptr,
+ unsigned long *ret)
{
- if ((unsigned long)ptr > TASK_SIZE - sizeof(unsigned long) ||
- ((unsigned long)ptr & 7))
- return -EFAULT;
-
- if (!probe_user_read(ret, ptr, sizeof(*ret)))
- return 0;
-
- return read_user_stack_slow(ptr, ret, 8);
+ return __read_user_stack(ptr, ret, sizeof(*ret));
}
/*
--
2.23.0
^ permalink raw reply related
* Re: [RFC/PATCH 2/3] pseries/kvm: Clear PSSCR[ESL|EC] bits before guest entry
From: David Gibson @ 2020-04-06 9:58 UTC (permalink / raw)
To: Gautham R Shenoy
Cc: Michael Neuling, Nicholas Piggin, Bharata B Rao, linuxppc-dev,
kvm-ppc, Vaidyanathan Srinivasan, linuxppc-dev
In-Reply-To: <20200403093103.GA20293@in.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 6206 bytes --]
On Fri, Apr 03, 2020 at 03:01:03PM +0530, Gautham R Shenoy wrote:
> On Fri, Apr 03, 2020 at 12:20:26PM +1000, Nicholas Piggin wrote:
> > Gautham R. Shenoy's on March 31, 2020 10:10 pm:
> > > From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
> > >
> > > ISA v3.0 allows the guest to execute a stop instruction. For this, the
> > > PSSCR[ESL|EC] bits need to be cleared by the hypervisor before
> > > scheduling in the guest vCPU.
> > >
> > > Currently we always schedule in a vCPU with PSSCR[ESL|EC] bits
> > > set. This patch changes the behaviour to enter the guest with
> > > PSSCR[ESL|EC] bits cleared. This is a RFC patch where we
> > > unconditionally clear these bits. Ideally this should be done
> > > conditionally on platforms where the guest stop instruction has no
> > > Bugs (starting POWER9 DD2.3).
> >
> > How will guests know that they can use this facility safely after your
> > series? You need both DD2.3 and a patched KVM.
>
>
> Yes, this is something that isn't addressed in this series (mentioned
> in the cover letter), which is a POC demonstrating that the stop0lite
> state in guest works.
>
> However, to answer your question, this is the scheme that I had in
> mind :
>
> OPAL:
> On Procs >= DD2.3 : we publish a dt-cpu-feature "idle-stop-guest"
>
> Hypervisor Kernel:
> 1. If "idle-stop-guest" dt-cpu-feature is discovered, then
> we set bool enable_guest_stop = true;
>
> 2. During KVM guest entry, clear PSSCR[ESL|EC] iff
> enable_guest_stop == true.
>
> 3. In kvm_vm_ioctl_check_extension(), for a new capability
> KVM_CAP_STOP, return true iff enable_guest_top == true.
>
> QEMU:
> Check with the hypervisor if KVM_CAP_STOP is present. If so,
> indicate the presence to the guest via device tree.
Nack. Presenting different capabilities to the guest depending on
host capabilities (rather than explicit options) is never ok. It
means that depending on the system you start on you may or may not be
able to migrate to other systems that you're supposed to be able to,
> Guest Kernel:
> Check for the presence of guest stop state support in
> device-tree. If available, enable the stop0lite in the cpuidle
> driver.
>
>
> We still have a challenge of migrating a guest which started on a
> hypervisor supporting guest stop state to a hypervisor without it.
> The target hypervisor should atleast have Patch 1 of this series, so
> that we don't crash the guest.
>
> >
> > >
> > > Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> > > ---
> > > arch/powerpc/kvm/book3s_hv.c | 2 +-
> > > arch/powerpc/kvm/book3s_hv_rmhandlers.S | 25 +++++++++++++------------
> > > 2 files changed, 14 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> > > index cdb7224..36d059a 100644
> > > --- a/arch/powerpc/kvm/book3s_hv.c
> > > +++ b/arch/powerpc/kvm/book3s_hv.c
> > > @@ -3424,7 +3424,7 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
> > > mtspr(SPRN_IC, vcpu->arch.ic);
> > > mtspr(SPRN_PID, vcpu->arch.pid);
> > >
> > > - mtspr(SPRN_PSSCR, vcpu->arch.psscr | PSSCR_EC |
> > > + mtspr(SPRN_PSSCR, (vcpu->arch.psscr & ~(PSSCR_EC | PSSCR_ESL)) |
> > > (local_paca->kvm_hstate.fake_suspend << PSSCR_FAKE_SUSPEND_LG));
> > >
> > > mtspr(SPRN_HFSCR, vcpu->arch.hfscr);
> > > diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> > > index dbc2fec..c2daec3 100644
> > > --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> > > +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> > > @@ -823,6 +823,18 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_207S)
> > > mtspr SPRN_PID, r7
> > > mtspr SPRN_WORT, r8
> > > BEGIN_FTR_SECTION
> > > + /* POWER9-only registers */
> > > + ld r5, VCPU_TID(r4)
> > > + ld r6, VCPU_PSSCR(r4)
> > > + lbz r8, HSTATE_FAKE_SUSPEND(r13)
> > > + lis r7, (PSSCR_EC | PSSCR_ESL)@h /* Allow guest to call stop */
> > > + andc r6, r6, r7
> > > + rldimi r6, r8, PSSCR_FAKE_SUSPEND_LG, 63 - PSSCR_FAKE_SUSPEND_LG
> > > + ld r7, VCPU_HFSCR(r4)
> > > + mtspr SPRN_TIDR, r5
> > > + mtspr SPRN_PSSCR, r6
> > > + mtspr SPRN_HFSCR, r7
> > > +FTR_SECTION_ELSE
> >
> > Why did you move these around? Just because the POWER9 section became
> > larger than the other?
>
> Yes.
>
> >
> > That's a real wart in the instruction patching implementation, I think
> > we can fix it by padding with nops in the macros.
> >
> > Can you just add the additional required nops to the top branch without
> > changing them around for this patch, so it's easier to see what's going
> > on? The end result will be the same after patching. Actually changing
> > these around can have a slight unintended consequence in that code that
> > runs before features were patched will execute the IF code. Not a
> > problem here, but another reason why the instruction patching
> > restriction is annoying.
>
> Sure, I will repost this patch with additional nops instead of
> moving them around.
>
> >
> > Thanks,
> > Nick
> >
> > > /* POWER8-only registers */
> > > ld r5, VCPU_TCSCR(r4)
> > > ld r6, VCPU_ACOP(r4)
> > > @@ -833,18 +845,7 @@ BEGIN_FTR_SECTION
> > > mtspr SPRN_CSIGR, r7
> > > mtspr SPRN_TACR, r8
> > > nop
> > > -FTR_SECTION_ELSE
> > > - /* POWER9-only registers */
> > > - ld r5, VCPU_TID(r4)
> > > - ld r6, VCPU_PSSCR(r4)
> > > - lbz r8, HSTATE_FAKE_SUSPEND(r13)
> > > - oris r6, r6, PSSCR_EC@h /* This makes stop trap to HV */
> > > - rldimi r6, r8, PSSCR_FAKE_SUSPEND_LG, 63 - PSSCR_FAKE_SUSPEND_LG
> > > - ld r7, VCPU_HFSCR(r4)
> > > - mtspr SPRN_TIDR, r5
> > > - mtspr SPRN_PSSCR, r6
> > > - mtspr SPRN_HFSCR, r7
> > > -ALT_FTR_SECTION_END_IFCLR(CPU_FTR_ARCH_300)
> > > +ALT_FTR_SECTION_END_IFSET(CPU_FTR_ARCH_300)
> > > 8:
> > >
> > > ld r5, VCPU_SPRG0(r4)
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ 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