diff mbox series

[v2,2/2,PowerPC] Allow use of SIMD in interrupts from kernel code

Message ID 20190514022308.32363-2-shawn@git.icu (mailing list archive)
State Superseded
Headers show
Series [v2,1/2,PowerPC] Add simd.h implementation | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success Successfully applied on branch next (8150a153c013aa2dd1ffae43370b89ac1347a7fb)
snowpatch_ozlabs/build-ppc64le warning Build succeeded but added 2 new sparse warnings
snowpatch_ozlabs/build-ppc64be warning Build succeeded but added 2 new sparse warnings
snowpatch_ozlabs/build-ppc64e fail build failed!
snowpatch_ozlabs/build-pmac32 warning Build succeeded but added 2 new sparse warnings
snowpatch_ozlabs/checkpatch fail total: 7 errors, 7 warnings, 5 checks, 141 lines checked

Commit Message

Shawn Landden May 14, 2019, 2:23 a.m. UTC 
This second patch is separate because it could be wrong,
like I am not sure about how kernel thread migration works,
and it is even allowing simd in preemptible kernel code.

Signed-off-by: Shawn Landden <shawn@git.icu>
---
 arch/powerpc/include/asm/switch_to.h | 15 ++-----
 arch/powerpc/kernel/process.c        | 60 ++++++++++++++++++++++++++--
 2 files changed, 59 insertions(+), 16 deletions(-)

Comments

Russell Currey May 14, 2019, 7:22 a.m. UTC | #1 
On Mon, 2019-05-13 at 23:23 -0300, Shawn Landden wrote:
> This second patch is separate because it could be wrong,
> like I am not sure about how kernel thread migration works,
> and it is even allowing simd in preemptible kernel code.
> 
> Signed-off-by: Shawn Landden <shawn@git.icu>
> ---

Hi Shawn,

This patch doesn't build on 64-bit embedded (ppc64e_defconfig):

arch/powerpc/kernel/process.c:194:13: error: 'interrupted_kernel_fpu_idle' defined but not used [-Werror=unused-function]
 static bool interrupted_kernel_fpu_idle(void)
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~

and otherwise adds two sparse warnings:

+arch/powerpc/kernel/process.c:356:13: warning: function 'disable_kernel_altivec' with external linkage has definition
+arch/powerpc/kernel/process.c:416:6: warning: symbol 'may_use_simd' was not declared. Should it be static?

There's also some style issues (spaces instead of tabs).

Reported by snowpatch (see https://patchwork.ozlabs.org/patch/1099181/)

- Russell
Shawn Landden May 14, 2019, 3:35 p.m. UTC | #2 
On Tue, May 14, 2019 at 2:22 AM Russell Currey <ruscur@russell.cc> wrote:
>
> On Mon, 2019-05-13 at 23:23 -0300, Shawn Landden wrote:
> > This second patch is separate because it could be wrong,
> > like I am not sure about how kernel thread migration works,
> > and it is even allowing simd in preemptible kernel code.
> >
> > Signed-off-by: Shawn Landden <shawn@git.icu>
> > ---
>
> Hi Shawn,
>
> This patch doesn't build on 64-bit embedded (ppc64e_defconfig):
>
> arch/powerpc/kernel/process.c:194:13: error: 'interrupted_kernel_fpu_idle' defined but not used [-Werror=unused-function]
>  static bool interrupted_kernel_fpu_idle(void)
>              ^~~~~~~~~~~~~~~~~~~~~~~~~~~
Thanks for noticing this. I knew what I needed to do, and this was
just sloppiness on my end.
>
> and otherwise adds two sparse warnings:
>
> +arch/powerpc/kernel/process.c:356:13: warning: function 'disable_kernel_altivec' with external linkage has definition
> +arch/powerpc/kernel/process.c:416:6: warning: symbol 'may_use_simd' was not declared. Should it be static?
This is the same problem as above.
>
> There's also some style issues (spaces instead of tabs).
Yes, I have to be careful using nano on a VPS.

New patch coming in 1 sec.
>
> Reported by snowpatch (see https://patchwork.ozlabs.org/patch/1099181/)
>
> - Russell
>
diff mbox series

Patch

diff --git a/arch/powerpc/include/asm/switch_to.h b/arch/powerpc/include/asm/switch_to.h
index 5b03d8a82..c79f7d24a 100644
--- a/arch/powerpc/include/asm/switch_to.h
+++ b/arch/powerpc/include/asm/switch_to.h
@@ -30,10 +30,7 @@  extern void enable_kernel_fp(void);
 extern void flush_fp_to_thread(struct task_struct *);
 extern void giveup_fpu(struct task_struct *);
 extern void save_fpu(struct task_struct *);
-static inline void disable_kernel_fp(void)
-{
-	msr_check_and_clear(MSR_FP);
-}
+extern void disable_kernel_fp(void);
 #else
 static inline void save_fpu(struct task_struct *t) { }
 static inline void flush_fp_to_thread(struct task_struct *t) { }
@@ -44,10 +41,7 @@  extern void enable_kernel_altivec(void);
 extern void flush_altivec_to_thread(struct task_struct *);
 extern void giveup_altivec(struct task_struct *);
 extern void save_altivec(struct task_struct *);
-static inline void disable_kernel_altivec(void)
-{
-	msr_check_and_clear(MSR_VEC);
-}
+extern void disable_kernel_altivec(void);
 #else
 static inline void save_altivec(struct task_struct *t) { }
 static inline void __giveup_altivec(struct task_struct *t) { }
@@ -56,10 +50,7 @@  static inline void __giveup_altivec(struct task_struct *t) { }
 #ifdef CONFIG_VSX
 extern void enable_kernel_vsx(void);
 extern void flush_vsx_to_thread(struct task_struct *);
-static inline void disable_kernel_vsx(void)
-{
-	msr_check_and_clear(MSR_FP|MSR_VEC|MSR_VSX);
-}
+extern void disable_kernel_vsx(void);
 #endif
 
 #ifdef CONFIG_SPE
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index ef534831f..d15f2cb51 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -170,6 +170,29 @@  void __msr_check_and_clear(unsigned long bits)
 EXPORT_SYMBOL(__msr_check_and_clear);
 
 #ifdef CONFIG_PPC_FPU
+/*
+ * Track whether the kernel is using the FPU state
+ * currently.
+ *
+ * This flag is used:
+ *
+ *   - by IRQ context code to potentially use the FPU
+ *     if it's unused.
+ *
+ *   - to debug kernel_fpu/altivec/vsx_begin()/end() correctness
+ */
+static DEFINE_PER_CPU(bool, in_kernel_fpu);
+
+static bool kernel_fpu_disabled(void)
+{
+        return this_cpu_read(in_kernel_fpu);
+}
+
+static bool interrupted_kernel_fpu_idle(void)
+{
+        return !kernel_fpu_disabled();
+}
+
 static void __giveup_fpu(struct task_struct *tsk)
 {
 	unsigned long msr;
@@ -230,7 +253,8 @@  void enable_kernel_fp(void)
 {
 	unsigned long cpumsr;
 
-	WARN_ON(preemptible());
+        WARN_ON_ONCE(this_cpu_read(in_kernel_fpu));
+        this_cpu_write(in_kernel_fpu, true);
 
 	cpumsr = msr_check_and_set(MSR_FP);
 
@@ -251,6 +275,15 @@  void enable_kernel_fp(void)
 }
 EXPORT_SYMBOL(enable_kernel_fp);
 
+void disable_kernel_fp(void)
+{
+        WARN_ON_ONCE(!this_cpu_read(in_kernel_fpu));
+        this_cpu_write(in_kernel_fpu, false);
+
+        msr_check_and_clear(MSR_FP);
+}
+EXPORT_SYMBOL(disable_kernel_fp);
+
 static int restore_fp(struct task_struct *tsk)
 {
 	if (tsk->thread.load_fp || tm_active_with_fp(tsk)) {
@@ -295,7 +328,8 @@  void enable_kernel_altivec(void)
 {
 	unsigned long cpumsr;
 
-	WARN_ON(preemptible());
+	WARN_ON_ONCE(this_cpu_read(in_kernel_fpu));
+	this_cpu_write(in_kernel_fpu, true);
 
 	cpumsr = msr_check_and_set(MSR_VEC);
 
@@ -316,6 +350,14 @@  void enable_kernel_altivec(void)
 }
 EXPORT_SYMBOL(enable_kernel_altivec);
 
+extern void disable_kernel_altivec(void)
+{
+	WARN_ON_ONCE(!this_cpu_read(in_kernel_fpu));
+	this_cpu_write(in_kernel_fpu, false);
+	msr_check_and_clear(MSR_VEC);
+}
+EXPORT_SYMBOL(disable_kernel_altivec);
+
 /*
  * Make sure the VMX/Altivec register state in the
  * the thread_struct is up to date for task tsk.
@@ -371,7 +413,8 @@  static bool interrupted_user_mode(void)
 bool may_use_simd(void)
 {
 	return !in_interrupt() ||
-		interrupted_user_mode();
+		interrupted_user_mode() ||
+		interrupted_kernel_fpu_idle();
 }
 EXPORT_SYMBOL(may_use_simd);
 
@@ -411,7 +454,8 @@  void enable_kernel_vsx(void)
 {
 	unsigned long cpumsr;
 
-	WARN_ON(preemptible());
+	WARN_ON_ONCE(this_cpu_read(in_kernel_fpu));
+	this_cpu_write(in_kernel_fpu, true);
 
 	cpumsr = msr_check_and_set(MSR_FP|MSR_VEC|MSR_VSX);
 
@@ -433,6 +477,14 @@  void enable_kernel_vsx(void)
 }
 EXPORT_SYMBOL(enable_kernel_vsx);
 
+void disable_kernel_vsx(void)
+{
+	WARN_ON_ONCE(!this_cpu_read(in_kernel_fpu));
+	this_cpu_write(in_kernel_fpu, false);
+	msr_check_and_clear(MSR_FP|MSR_VEC|MSR_VSX);
+}
+EXPORT_SYMBOL(disable_kernel_vsx);
+
 void flush_vsx_to_thread(struct task_struct *tsk)
 {
 	if (tsk->thread.regs) {