All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] sched, x86, arm64: PREEMPT_RT, FPU and preemption
@ 2021-07-22 17:51 ` Valentin Schneider
  0 siblings, 0 replies; 16+ messages in thread
From: Valentin Schneider @ 2021-07-22 17:51 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, linux-rt-users, x86
  Cc: Catalin Marinas, Will Deacon, Thomas Gleixner, Steven Rostedt,
	Daniel Bristot de Oliveira, Peter Zijlstra (Intel),
	Ingo Molnar, Borislav Petkov, H. Peter Anvin, Mark Brown,
	Dave Martin, Ard Biesheuvel

Hi folks,

This stems from some more v5.13-rt1 breakage on arm64. This time per-CPU access
safety isn't sufficient, we really need to keep preemption disabled.

In a happy accident I stumbled on

  cba08c5dc6dc ("x86/fpu: Make kernel FPU protection RT friendly")

so I packaged what was done there into some common helpers and plastered them
over the problematic areas.

Cheers,
Valentin

Valentin Schneider (3):
  sched/preempt: Introduce preempt_{enable, disable}_bh()
  x86/fpu: Make FPU protection reuse common helper
  arm64/fpsimd: Fix FPSIMD context handling vs PREEMPT_RT

 arch/arm64/kernel/fpsimd.c     |  6 ++++--
 arch/x86/include/asm/fpu/api.h | 19 ++-----------------
 include/linux/bottom_half.h    | 26 ++++++++++++++++++++++++++
 3 files changed, 32 insertions(+), 19 deletions(-)

--
2.25.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 0/3] sched, x86, arm64: PREEMPT_RT, FPU and preemption
@ 2021-07-22 17:51 ` Valentin Schneider
  0 siblings, 0 replies; 16+ messages in thread
From: Valentin Schneider @ 2021-07-22 17:51 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, linux-rt-users, x86
  Cc: Catalin Marinas, Will Deacon, Thomas Gleixner, Steven Rostedt,
	Daniel Bristot de Oliveira, Peter Zijlstra (Intel),
	Ingo Molnar, Borislav Petkov, H. Peter Anvin, Mark Brown,
	Dave Martin, Ard Biesheuvel

Hi folks,

This stems from some more v5.13-rt1 breakage on arm64. This time per-CPU access
safety isn't sufficient, we really need to keep preemption disabled.

In a happy accident I stumbled on

  cba08c5dc6dc ("x86/fpu: Make kernel FPU protection RT friendly")

so I packaged what was done there into some common helpers and plastered them
over the problematic areas.

Cheers,
Valentin

Valentin Schneider (3):
  sched/preempt: Introduce preempt_{enable, disable}_bh()
  x86/fpu: Make FPU protection reuse common helper
  arm64/fpsimd: Fix FPSIMD context handling vs PREEMPT_RT

 arch/arm64/kernel/fpsimd.c     |  6 ++++--
 arch/x86/include/asm/fpu/api.h | 19 ++-----------------
 include/linux/bottom_half.h    | 26 ++++++++++++++++++++++++++
 3 files changed, 32 insertions(+), 19 deletions(-)

--
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/3] sched/preempt: Introduce preempt_{enable, disable}_bh()
  2021-07-22 17:51 ` Valentin Schneider
@ 2021-07-22 17:51   ` Valentin Schneider
  -1 siblings, 0 replies; 16+ messages in thread
From: Valentin Schneider @ 2021-07-22 17:51 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, linux-rt-users, x86
  Cc: Catalin Marinas, Will Deacon, Thomas Gleixner, Steven Rostedt,
	Daniel Bristot de Oliveira, Peter Zijlstra (Intel),
	Ingo Molnar, Borislav Petkov, H. Peter Anvin, Mark Brown,
	Dave Martin, Ard Biesheuvel

As pointed out by commit:

  cba08c5dc6dc ("x86/fpu: Make kernel FPU protection RT friendly")

local_bh_disable() is not sufficient under CONFIG_PREEMPT_RT for FPU
manipulation. This also affects at least arm64, so package the preemption /
softirq enablement handling into a pair of helpers.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
 include/linux/bottom_half.h | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/include/linux/bottom_half.h b/include/linux/bottom_half.h
index eed86eb0a1de..dd98d8c1eb0a 100644
--- a/include/linux/bottom_half.h
+++ b/include/linux/bottom_half.h
@@ -19,6 +19,24 @@ static inline void local_bh_disable(void)
 	__local_bh_disable_ip(_THIS_IP_, SOFTIRQ_DISABLE_OFFSET);
 }
 
+/*
+ * local_bh_disable() protects against both preemption and soft interrupts
+ * on !RT kernels.
+ *
+ * On RT kernels local_bh_disable() is not sufficient because it only
+ * serializes soft interrupt related sections via a local lock, but stays
+ * preemptible. Disabling preemption is the right choice here as bottom
+ * half processing is always in thread context on RT kernels so it
+ * implicitly prevents bottom half processing as well.
+ */
+static inline void preempt_disable_bh(void)
+{
+	if (IS_ENABLED(CONFIG_PREEMPT_RT))
+		preempt_disable();
+	else
+		local_bh_disable();
+}
+
 extern void _local_bh_enable(void);
 extern void __local_bh_enable_ip(unsigned long ip, unsigned int cnt);
 
@@ -32,6 +50,14 @@ static inline void local_bh_enable(void)
 	__local_bh_enable_ip(_THIS_IP_, SOFTIRQ_DISABLE_OFFSET);
 }
 
+static inline void preempt_enable_bh(void)
+{
+	if (IS_ENABLED(CONFIG_PREEMPT_RT))
+		preempt_enable();
+	else
+		local_bh_enable();
+}
+
 #ifdef CONFIG_PREEMPT_RT
 extern bool local_bh_blocked(void);
 #else
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 1/3] sched/preempt: Introduce preempt_{enable, disable}_bh()
@ 2021-07-22 17:51   ` Valentin Schneider
  0 siblings, 0 replies; 16+ messages in thread
From: Valentin Schneider @ 2021-07-22 17:51 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, linux-rt-users, x86
  Cc: Catalin Marinas, Will Deacon, Thomas Gleixner, Steven Rostedt,
	Daniel Bristot de Oliveira, Peter Zijlstra (Intel),
	Ingo Molnar, Borislav Petkov, H. Peter Anvin, Mark Brown,
	Dave Martin, Ard Biesheuvel

As pointed out by commit:

  cba08c5dc6dc ("x86/fpu: Make kernel FPU protection RT friendly")

local_bh_disable() is not sufficient under CONFIG_PREEMPT_RT for FPU
manipulation. This also affects at least arm64, so package the preemption /
softirq enablement handling into a pair of helpers.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
 include/linux/bottom_half.h | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/include/linux/bottom_half.h b/include/linux/bottom_half.h
index eed86eb0a1de..dd98d8c1eb0a 100644
--- a/include/linux/bottom_half.h
+++ b/include/linux/bottom_half.h
@@ -19,6 +19,24 @@ static inline void local_bh_disable(void)
 	__local_bh_disable_ip(_THIS_IP_, SOFTIRQ_DISABLE_OFFSET);
 }
 
+/*
+ * local_bh_disable() protects against both preemption and soft interrupts
+ * on !RT kernels.
+ *
+ * On RT kernels local_bh_disable() is not sufficient because it only
+ * serializes soft interrupt related sections via a local lock, but stays
+ * preemptible. Disabling preemption is the right choice here as bottom
+ * half processing is always in thread context on RT kernels so it
+ * implicitly prevents bottom half processing as well.
+ */
+static inline void preempt_disable_bh(void)
+{
+	if (IS_ENABLED(CONFIG_PREEMPT_RT))
+		preempt_disable();
+	else
+		local_bh_disable();
+}
+
 extern void _local_bh_enable(void);
 extern void __local_bh_enable_ip(unsigned long ip, unsigned int cnt);
 
@@ -32,6 +50,14 @@ static inline void local_bh_enable(void)
 	__local_bh_enable_ip(_THIS_IP_, SOFTIRQ_DISABLE_OFFSET);
 }
 
+static inline void preempt_enable_bh(void)
+{
+	if (IS_ENABLED(CONFIG_PREEMPT_RT))
+		preempt_enable();
+	else
+		local_bh_enable();
+}
+
 #ifdef CONFIG_PREEMPT_RT
 extern bool local_bh_blocked(void);
 #else
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/3] x86/fpu: Make FPU protection reuse common helper
  2021-07-22 17:51 ` Valentin Schneider
@ 2021-07-22 17:51   ` Valentin Schneider
  -1 siblings, 0 replies; 16+ messages in thread
From: Valentin Schneider @ 2021-07-22 17:51 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, linux-rt-users, x86
  Cc: Catalin Marinas, Will Deacon, Thomas Gleixner, Steven Rostedt,
	Daniel Bristot de Oliveira, Peter Zijlstra (Intel),
	Ingo Molnar, Borislav Petkov, H. Peter Anvin, Mark Brown,
	Dave Martin, Ard Biesheuvel

The newly-introduced preempt_{enable, disable}_bh() helpers are an exact
stand-in for fpregs_{lock, unlock}(). Use them there.

No change in behaviour intended.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
 arch/x86/include/asm/fpu/api.h | 19 ++-----------------
 1 file changed, 2 insertions(+), 17 deletions(-)

diff --git a/arch/x86/include/asm/fpu/api.h b/arch/x86/include/asm/fpu/api.h
index 62cf3e4c06fb..ffebb9316bfd 100644
--- a/arch/x86/include/asm/fpu/api.h
+++ b/arch/x86/include/asm/fpu/api.h
@@ -54,31 +54,16 @@ static inline void kernel_fpu_begin(void)
  * fpu->state and set TIF_NEED_FPU_LOAD leaving CPU's FPU registers in
  * a random state.
  *
- * local_bh_disable() protects against both preemption and soft interrupts
- * on !RT kernels.
- *
- * On RT kernels local_bh_disable() is not sufficient because it only
- * serializes soft interrupt related sections via a local lock, but stays
- * preemptible. Disabling preemption is the right choice here as bottom
- * half processing is always in thread context on RT kernels so it
- * implicitly prevents bottom half processing as well.
- *
  * Disabling preemption also serializes against kernel_fpu_begin().
  */
 static inline void fpregs_lock(void)
 {
-	if (!IS_ENABLED(CONFIG_PREEMPT_RT))
-		local_bh_disable();
-	else
-		preempt_disable();
+	preempt_disable_bh();
 }
 
 static inline void fpregs_unlock(void)
 {
-	if (!IS_ENABLED(CONFIG_PREEMPT_RT))
-		local_bh_enable();
-	else
-		preempt_enable();
+	preempt_enable_bh();
 }
 
 #ifdef CONFIG_X86_DEBUG_FPU
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/3] x86/fpu: Make FPU protection reuse common helper
@ 2021-07-22 17:51   ` Valentin Schneider
  0 siblings, 0 replies; 16+ messages in thread
From: Valentin Schneider @ 2021-07-22 17:51 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, linux-rt-users, x86
  Cc: Catalin Marinas, Will Deacon, Thomas Gleixner, Steven Rostedt,
	Daniel Bristot de Oliveira, Peter Zijlstra (Intel),
	Ingo Molnar, Borislav Petkov, H. Peter Anvin, Mark Brown,
	Dave Martin, Ard Biesheuvel

The newly-introduced preempt_{enable, disable}_bh() helpers are an exact
stand-in for fpregs_{lock, unlock}(). Use them there.

No change in behaviour intended.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
 arch/x86/include/asm/fpu/api.h | 19 ++-----------------
 1 file changed, 2 insertions(+), 17 deletions(-)

diff --git a/arch/x86/include/asm/fpu/api.h b/arch/x86/include/asm/fpu/api.h
index 62cf3e4c06fb..ffebb9316bfd 100644
--- a/arch/x86/include/asm/fpu/api.h
+++ b/arch/x86/include/asm/fpu/api.h
@@ -54,31 +54,16 @@ static inline void kernel_fpu_begin(void)
  * fpu->state and set TIF_NEED_FPU_LOAD leaving CPU's FPU registers in
  * a random state.
  *
- * local_bh_disable() protects against both preemption and soft interrupts
- * on !RT kernels.
- *
- * On RT kernels local_bh_disable() is not sufficient because it only
- * serializes soft interrupt related sections via a local lock, but stays
- * preemptible. Disabling preemption is the right choice here as bottom
- * half processing is always in thread context on RT kernels so it
- * implicitly prevents bottom half processing as well.
- *
  * Disabling preemption also serializes against kernel_fpu_begin().
  */
 static inline void fpregs_lock(void)
 {
-	if (!IS_ENABLED(CONFIG_PREEMPT_RT))
-		local_bh_disable();
-	else
-		preempt_disable();
+	preempt_disable_bh();
 }
 
 static inline void fpregs_unlock(void)
 {
-	if (!IS_ENABLED(CONFIG_PREEMPT_RT))
-		local_bh_enable();
-	else
-		preempt_enable();
+	preempt_enable_bh();
 }
 
 #ifdef CONFIG_X86_DEBUG_FPU
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/3] arm64/fpsimd: Fix FPSIMD context handling vs PREEMPT_RT
  2021-07-22 17:51 ` Valentin Schneider
@ 2021-07-22 17:51   ` Valentin Schneider
  -1 siblings, 0 replies; 16+ messages in thread
From: Valentin Schneider @ 2021-07-22 17:51 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, linux-rt-users, x86
  Cc: Catalin Marinas, Will Deacon, Thomas Gleixner, Steven Rostedt,
	Daniel Bristot de Oliveira, Peter Zijlstra (Intel),
	Ingo Molnar, Borislav Petkov, H. Peter Anvin, Mark Brown,
	Dave Martin, Ard Biesheuvel

Running v5.13-rt1 on my arm64 Juno board triggers:

[   11.337654] WARNING: CPU: 4 PID: 1 at arch/arm64/kernel/fpsimd.c:296 task_fpsimd_load (arch/arm64/kernel/fpsimd.c:296 (discriminator 1))
[   11.337692] Modules linked in:
[   11.337705] CPU: 4 PID: 1 Comm: init Not tainted 5.13.0-rt1 #52
[   11.337719] Hardware name: ARM Juno development board (r0) (DT)
[   11.337727] pstate: 40000005 (nZcv daif -PAN -UAO -TCO BTYPE=--)
[   11.337740] pc : task_fpsimd_load (arch/arm64/kernel/fpsimd.c:296 (discriminator 1))
[   11.337755] lr : task_fpsimd_load (arch/arm64/kernel/fpsimd.c:296 (discriminator 3))
[   11.337769] sp : ffff800012f4bdd0
[   11.337775] x29: ffff800012f4bdd0 x28: ffff000800160000 x27: 0000000000000000
[   11.337797] x26: 0000000000000000 x25: 0000000000000000 x24: ffff0008001606f0
[   11.337816] x23: ffff000800160000 x22: ffff000800160700 x21: ffff000800160000
[   11.337837] x20: ffff800012f4beb0 x19: 0000000000000008 x18: 000000000000c9a0
[   11.337857] x17: 00000000ae3495d5 x16: 000000000000c9a0 x15: ffff80001240e128
[   11.337878] x14: ffff8000124b0128 x13: 000000000000000a x12: ffff80001205e5f0
[   11.337898] x11: 0000000000000000 x10: ffff800011a37d28 x9 : 0000000000000050
[   11.337917] x8 : ffff000800160000 x7 : 0000000000000002 x6 : 0000000000000000
[   11.337937] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
[   11.337956] x2 : 0000000000000008 x1 : 0000000000000000 x0 : 0000000000000000
[   11.337975] Call trace:
[   11.337980] task_fpsimd_load (arch/arm64/kernel/fpsimd.c:296 (discriminator 1))
[   11.337996] fpsimd_restore_current_state (arch/arm64/kernel/fpsimd.c:1186)
[   11.338012] do_notify_resume (./arch/arm64/include/asm/daifflags.h:28 arch/arm64/kernel/signal.c:947)
[   11.338032] work_pending (arch/arm64/kernel/entry.S:839)
[   11.338045] irq event stamp: 1228377
[   11.338051] hardirqs last enabled at (1228375): _raw_spin_unlock_irqrestore (./include/linux/spinlock_api_smp.h:160 kernel/locking/spinlock.c:194)
[   11.338076] hardirqs last disabled at (1228377): el1_dbg (arch/arm64/kernel/entry-common.c:144 arch/arm64/kernel/entry-common.c:234)
[   11.338098] softirqs last enabled at (1227024): __local_bh_enable_ip (./arch/arm64/include/asm/irqflags.h:85 kernel/softirq.c:262)
[   11.338121] softirqs last disabled at (1228376): fpsimd_restore_current_state (./include/linux/bottom_half.h:19 arch/arm64/kernel/fpsimd.c:183 arch/arm64/kernel/fpsimd.c:1182)

This is caused by local_bh_disable() not disabling preemption under
CONFIG_PREEMPT_RT, which fails have_cpu_fpsimd_context(). The per-CPU
access safety proved by the CONFIG_PREEMPT_RT version of local_bh_disable()
is not sufficient here, as we end up with both preemption and IRQs enabled
when running do_notify_resume(). This means we can hit:

  el0_sync
  `\
    do_notify_resume()
    `\
      fpsimd_restore_current_state()

  ~~> el1_irq
      `\
	preempt_schedule_irq()
	`\
	  fpsimd_thread_switch()

IOW we *really* need to disable preemption here, even on CONFIG_PREEMPT_RT.

The preempt_{enable, disable}_bh() helpers exist to handle this exact case,
so use them here. This is spiritually close to:

  cba08c5dc6dc ("x86/fpu: Make kernel FPU protection RT friendly")

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
 arch/arm64/kernel/fpsimd.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 9bf86cd7b605..0c66ea0dd97e 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -177,10 +177,12 @@ static void __get_cpu_fpsimd_context(void)
  *
  * The double-underscore version must only be called if you know the task
  * can't be preempted.
+ *
+ * Disabling preemption prevents nesting via fpsimd_thread_switch().
  */
 static void get_cpu_fpsimd_context(void)
 {
-	local_bh_disable();
+	preempt_disable_bh();
 	__get_cpu_fpsimd_context();
 }
 
@@ -201,7 +203,7 @@ static void __put_cpu_fpsimd_context(void)
 static void put_cpu_fpsimd_context(void)
 {
 	__put_cpu_fpsimd_context();
-	local_bh_enable();
+	preempt_enable_bh();
 }
 
 static bool have_cpu_fpsimd_context(void)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/3] arm64/fpsimd: Fix FPSIMD context handling vs PREEMPT_RT
@ 2021-07-22 17:51   ` Valentin Schneider
  0 siblings, 0 replies; 16+ messages in thread
From: Valentin Schneider @ 2021-07-22 17:51 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, linux-rt-users, x86
  Cc: Catalin Marinas, Will Deacon, Thomas Gleixner, Steven Rostedt,
	Daniel Bristot de Oliveira, Peter Zijlstra (Intel),
	Ingo Molnar, Borislav Petkov, H. Peter Anvin, Mark Brown,
	Dave Martin, Ard Biesheuvel

Running v5.13-rt1 on my arm64 Juno board triggers:

[   11.337654] WARNING: CPU: 4 PID: 1 at arch/arm64/kernel/fpsimd.c:296 task_fpsimd_load (arch/arm64/kernel/fpsimd.c:296 (discriminator 1))
[   11.337692] Modules linked in:
[   11.337705] CPU: 4 PID: 1 Comm: init Not tainted 5.13.0-rt1 #52
[   11.337719] Hardware name: ARM Juno development board (r0) (DT)
[   11.337727] pstate: 40000005 (nZcv daif -PAN -UAO -TCO BTYPE=--)
[   11.337740] pc : task_fpsimd_load (arch/arm64/kernel/fpsimd.c:296 (discriminator 1))
[   11.337755] lr : task_fpsimd_load (arch/arm64/kernel/fpsimd.c:296 (discriminator 3))
[   11.337769] sp : ffff800012f4bdd0
[   11.337775] x29: ffff800012f4bdd0 x28: ffff000800160000 x27: 0000000000000000
[   11.337797] x26: 0000000000000000 x25: 0000000000000000 x24: ffff0008001606f0
[   11.337816] x23: ffff000800160000 x22: ffff000800160700 x21: ffff000800160000
[   11.337837] x20: ffff800012f4beb0 x19: 0000000000000008 x18: 000000000000c9a0
[   11.337857] x17: 00000000ae3495d5 x16: 000000000000c9a0 x15: ffff80001240e128
[   11.337878] x14: ffff8000124b0128 x13: 000000000000000a x12: ffff80001205e5f0
[   11.337898] x11: 0000000000000000 x10: ffff800011a37d28 x9 : 0000000000000050
[   11.337917] x8 : ffff000800160000 x7 : 0000000000000002 x6 : 0000000000000000
[   11.337937] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
[   11.337956] x2 : 0000000000000008 x1 : 0000000000000000 x0 : 0000000000000000
[   11.337975] Call trace:
[   11.337980] task_fpsimd_load (arch/arm64/kernel/fpsimd.c:296 (discriminator 1))
[   11.337996] fpsimd_restore_current_state (arch/arm64/kernel/fpsimd.c:1186)
[   11.338012] do_notify_resume (./arch/arm64/include/asm/daifflags.h:28 arch/arm64/kernel/signal.c:947)
[   11.338032] work_pending (arch/arm64/kernel/entry.S:839)
[   11.338045] irq event stamp: 1228377
[   11.338051] hardirqs last enabled at (1228375): _raw_spin_unlock_irqrestore (./include/linux/spinlock_api_smp.h:160 kernel/locking/spinlock.c:194)
[   11.338076] hardirqs last disabled at (1228377): el1_dbg (arch/arm64/kernel/entry-common.c:144 arch/arm64/kernel/entry-common.c:234)
[   11.338098] softirqs last enabled at (1227024): __local_bh_enable_ip (./arch/arm64/include/asm/irqflags.h:85 kernel/softirq.c:262)
[   11.338121] softirqs last disabled at (1228376): fpsimd_restore_current_state (./include/linux/bottom_half.h:19 arch/arm64/kernel/fpsimd.c:183 arch/arm64/kernel/fpsimd.c:1182)

This is caused by local_bh_disable() not disabling preemption under
CONFIG_PREEMPT_RT, which fails have_cpu_fpsimd_context(). The per-CPU
access safety proved by the CONFIG_PREEMPT_RT version of local_bh_disable()
is not sufficient here, as we end up with both preemption and IRQs enabled
when running do_notify_resume(). This means we can hit:

  el0_sync
  `\
    do_notify_resume()
    `\
      fpsimd_restore_current_state()

  ~~> el1_irq
      `\
	preempt_schedule_irq()
	`\
	  fpsimd_thread_switch()

IOW we *really* need to disable preemption here, even on CONFIG_PREEMPT_RT.

The preempt_{enable, disable}_bh() helpers exist to handle this exact case,
so use them here. This is spiritually close to:

  cba08c5dc6dc ("x86/fpu: Make kernel FPU protection RT friendly")

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
 arch/arm64/kernel/fpsimd.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 9bf86cd7b605..0c66ea0dd97e 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -177,10 +177,12 @@ static void __get_cpu_fpsimd_context(void)
  *
  * The double-underscore version must only be called if you know the task
  * can't be preempted.
+ *
+ * Disabling preemption prevents nesting via fpsimd_thread_switch().
  */
 static void get_cpu_fpsimd_context(void)
 {
-	local_bh_disable();
+	preempt_disable_bh();
 	__get_cpu_fpsimd_context();
 }
 
@@ -201,7 +203,7 @@ static void __put_cpu_fpsimd_context(void)
 static void put_cpu_fpsimd_context(void)
 {
 	__put_cpu_fpsimd_context();
-	local_bh_enable();
+	preempt_enable_bh();
 }
 
 static bool have_cpu_fpsimd_context(void)
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] arm64/fpsimd: Fix FPSIMD context handling vs PREEMPT_RT
  2021-07-22 17:51   ` Valentin Schneider
@ 2021-07-22 18:32     ` Mark Brown
  -1 siblings, 0 replies; 16+ messages in thread
From: Mark Brown @ 2021-07-22 18:32 UTC (permalink / raw)
  To: Valentin Schneider
  Cc: linux-kernel, linux-arm-kernel, linux-rt-users, x86,
	Catalin Marinas, Will Deacon, Thomas Gleixner, Steven Rostedt,
	Daniel Bristot de Oliveira, Peter Zijlstra (Intel),
	Ingo Molnar, Borislav Petkov, H. Peter Anvin, Dave Martin,
	Ard Biesheuvel

[-- Attachment #1: Type: text/plain, Size: 1223 bytes --]

On Thu, Jul 22, 2021 at 06:51:57PM +0100, Valentin Schneider wrote:
> Running v5.13-rt1 on my arm64 Juno board triggers:

Acked-by: Mark Brown <broonie@kernel.org>

> [   11.337654] WARNING: CPU: 4 PID: 1 at arch/arm64/kernel/fpsimd.c:296 task_fpsimd_load (arch/arm64/kernel/fpsimd.c:296 (discriminator 1))
> [   11.337692] Modules linked in:
> [   11.337705] CPU: 4 PID: 1 Comm: init Not tainted 5.13.0-rt1 #52
> [   11.337719] Hardware name: ARM Juno development board (r0) (DT)
> [   11.337727] pstate: 40000005 (nZcv daif -PAN -UAO -TCO BTYPE=--)
> [   11.337740] pc : task_fpsimd_load (arch/arm64/kernel/fpsimd.c:296 (discriminator 1))
> [   11.337755] lr : task_fpsimd_load (arch/arm64/kernel/fpsimd.c:296 (discriminator 3))
> [   11.337769] sp : ffff800012f4bdd0
> [   11.337775] x29: ffff800012f4bdd0 x28: ffff000800160000 x27: 0000000000000000
> [   11.337797] x26: 0000000000000000 x25: 0000000000000000 x24: ffff0008001606f0
> [   11.337816] x23: ffff000800160000 x22: ffff000800160700 x21: ffff000800160000

For readability it's probably better to not include the entire register
contents here - the backtrace ends up about as large as the actual
changelog, a good chunk of which is just the register contents.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] arm64/fpsimd: Fix FPSIMD context handling vs PREEMPT_RT
@ 2021-07-22 18:32     ` Mark Brown
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Brown @ 2021-07-22 18:32 UTC (permalink / raw)
  To: Valentin Schneider
  Cc: linux-kernel, linux-arm-kernel, linux-rt-users, x86,
	Catalin Marinas, Will Deacon, Thomas Gleixner, Steven Rostedt,
	Daniel Bristot de Oliveira, Peter Zijlstra (Intel),
	Ingo Molnar, Borislav Petkov, H. Peter Anvin, Dave Martin,
	Ard Biesheuvel


[-- Attachment #1.1: Type: text/plain, Size: 1223 bytes --]

On Thu, Jul 22, 2021 at 06:51:57PM +0100, Valentin Schneider wrote:
> Running v5.13-rt1 on my arm64 Juno board triggers:

Acked-by: Mark Brown <broonie@kernel.org>

> [   11.337654] WARNING: CPU: 4 PID: 1 at arch/arm64/kernel/fpsimd.c:296 task_fpsimd_load (arch/arm64/kernel/fpsimd.c:296 (discriminator 1))
> [   11.337692] Modules linked in:
> [   11.337705] CPU: 4 PID: 1 Comm: init Not tainted 5.13.0-rt1 #52
> [   11.337719] Hardware name: ARM Juno development board (r0) (DT)
> [   11.337727] pstate: 40000005 (nZcv daif -PAN -UAO -TCO BTYPE=--)
> [   11.337740] pc : task_fpsimd_load (arch/arm64/kernel/fpsimd.c:296 (discriminator 1))
> [   11.337755] lr : task_fpsimd_load (arch/arm64/kernel/fpsimd.c:296 (discriminator 3))
> [   11.337769] sp : ffff800012f4bdd0
> [   11.337775] x29: ffff800012f4bdd0 x28: ffff000800160000 x27: 0000000000000000
> [   11.337797] x26: 0000000000000000 x25: 0000000000000000 x24: ffff0008001606f0
> [   11.337816] x23: ffff000800160000 x22: ffff000800160700 x21: ffff000800160000

For readability it's probably better to not include the entire register
contents here - the backtrace ends up about as large as the actual
changelog, a good chunk of which is just the register contents.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/3] sched, x86, arm64: PREEMPT_RT, FPU and preemption
  2021-07-22 17:51 ` Valentin Schneider
@ 2021-07-22 20:11   ` Andrew Halaney
  -1 siblings, 0 replies; 16+ messages in thread
From: Andrew Halaney @ 2021-07-22 20:11 UTC (permalink / raw)
  To: Valentin Schneider
  Cc: linux-kernel, linux-arm-kernel, linux-rt-users, x86,
	Catalin Marinas, Will Deacon, Thomas Gleixner, Steven Rostedt,
	Daniel Bristot de Oliveira, Peter Zijlstra (Intel),
	Ingo Molnar, Borislav Petkov, H. Peter Anvin, Mark Brown,
	Dave Martin, Ard Biesheuvel

On Thu, Jul 22, 2021 at 06:51:54PM +0100, Valentin Schneider wrote:
> Hi folks,
> 
> This stems from some more v5.13-rt1 breakage on arm64. This time per-CPU access
> safety isn't sufficient, we really need to keep preemption disabled.
> 
> In a happy accident I stumbled on
> 
>   cba08c5dc6dc ("x86/fpu: Make kernel FPU protection RT friendly")
> 
> so I packaged what was done there into some common helpers and plastered them
> over the problematic areas.
> 
> Cheers,
> Valentin
> 
> Valentin Schneider (3):
>   sched/preempt: Introduce preempt_{enable, disable}_bh()
>   x86/fpu: Make FPU protection reuse common helper
>   arm64/fpsimd: Fix FPSIMD context handling vs PREEMPT_RT
> 
>  arch/arm64/kernel/fpsimd.c     |  6 ++++--
>  arch/x86/include/asm/fpu/api.h | 19 ++-----------------
>  include/linux/bottom_half.h    | 26 ++++++++++++++++++++++++++
>  3 files changed, 32 insertions(+), 19 deletions(-)
> 
> --
> 2.25.1
> 

I'm an outsider on all of this, but this series makes sense and looks
good to me.

Reviewed-by: Andrew Halaney <ahalaney@redhat.com>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/3] sched, x86, arm64: PREEMPT_RT, FPU and preemption
@ 2021-07-22 20:11   ` Andrew Halaney
  0 siblings, 0 replies; 16+ messages in thread
From: Andrew Halaney @ 2021-07-22 20:11 UTC (permalink / raw)
  To: Valentin Schneider
  Cc: linux-kernel, linux-arm-kernel, linux-rt-users, x86,
	Catalin Marinas, Will Deacon, Thomas Gleixner, Steven Rostedt,
	Daniel Bristot de Oliveira, Peter Zijlstra (Intel),
	Ingo Molnar, Borislav Petkov, H. Peter Anvin, Mark Brown,
	Dave Martin, Ard Biesheuvel

On Thu, Jul 22, 2021 at 06:51:54PM +0100, Valentin Schneider wrote:
> Hi folks,
> 
> This stems from some more v5.13-rt1 breakage on arm64. This time per-CPU access
> safety isn't sufficient, we really need to keep preemption disabled.
> 
> In a happy accident I stumbled on
> 
>   cba08c5dc6dc ("x86/fpu: Make kernel FPU protection RT friendly")
> 
> so I packaged what was done there into some common helpers and plastered them
> over the problematic areas.
> 
> Cheers,
> Valentin
> 
> Valentin Schneider (3):
>   sched/preempt: Introduce preempt_{enable, disable}_bh()
>   x86/fpu: Make FPU protection reuse common helper
>   arm64/fpsimd: Fix FPSIMD context handling vs PREEMPT_RT
> 
>  arch/arm64/kernel/fpsimd.c     |  6 ++++--
>  arch/x86/include/asm/fpu/api.h | 19 ++-----------------
>  include/linux/bottom_half.h    | 26 ++++++++++++++++++++++++++
>  3 files changed, 32 insertions(+), 19 deletions(-)
> 
> --
> 2.25.1
> 

I'm an outsider on all of this, but this series makes sense and looks
good to me.

Reviewed-by: Andrew Halaney <ahalaney@redhat.com>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] arm64/fpsimd: Fix FPSIMD context handling vs PREEMPT_RT
  2021-07-22 18:32     ` Mark Brown
@ 2021-07-23 11:07       ` Valentin Schneider
  -1 siblings, 0 replies; 16+ messages in thread
From: Valentin Schneider @ 2021-07-23 11:07 UTC (permalink / raw)
  To: Mark Brown
  Cc: linux-kernel, linux-arm-kernel, linux-rt-users, x86,
	Catalin Marinas, Will Deacon, Thomas Gleixner, Steven Rostedt,
	Daniel Bristot de Oliveira, Peter Zijlstra (Intel),
	Ingo Molnar, Borislav Petkov, H. Peter Anvin, Dave Martin,
	Ard Biesheuvel

On 22/07/21 19:32, Mark Brown wrote:
> On Thu, Jul 22, 2021 at 06:51:57PM +0100, Valentin Schneider wrote:
>> Running v5.13-rt1 on my arm64 Juno board triggers:
>
> Acked-by: Mark Brown <broonie@kernel.org>
>

Thanks!

>> [   11.337654] WARNING: CPU: 4 PID: 1 at arch/arm64/kernel/fpsimd.c:296 task_fpsimd_load (arch/arm64/kernel/fpsimd.c:296 (discriminator 1))
>> [   11.337692] Modules linked in:
>> [   11.337705] CPU: 4 PID: 1 Comm: init Not tainted 5.13.0-rt1 #52
>> [   11.337719] Hardware name: ARM Juno development board (r0) (DT)
>> [   11.337727] pstate: 40000005 (nZcv daif -PAN -UAO -TCO BTYPE=--)
>> [   11.337740] pc : task_fpsimd_load (arch/arm64/kernel/fpsimd.c:296 (discriminator 1))
>> [   11.337755] lr : task_fpsimd_load (arch/arm64/kernel/fpsimd.c:296 (discriminator 3))
>> [   11.337769] sp : ffff800012f4bdd0
>> [   11.337775] x29: ffff800012f4bdd0 x28: ffff000800160000 x27: 0000000000000000
>> [   11.337797] x26: 0000000000000000 x25: 0000000000000000 x24: ffff0008001606f0
>> [   11.337816] x23: ffff000800160000 x22: ffff000800160700 x21: ffff000800160000
>
> For readability it's probably better to not include the entire register
> contents here - the backtrace ends up about as large as the actual
> changelog, a good chunk of which is just the register contents.

Noted.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] arm64/fpsimd: Fix FPSIMD context handling vs PREEMPT_RT
@ 2021-07-23 11:07       ` Valentin Schneider
  0 siblings, 0 replies; 16+ messages in thread
From: Valentin Schneider @ 2021-07-23 11:07 UTC (permalink / raw)
  To: Mark Brown
  Cc: linux-kernel, linux-arm-kernel, linux-rt-users, x86,
	Catalin Marinas, Will Deacon, Thomas Gleixner, Steven Rostedt,
	Daniel Bristot de Oliveira, Peter Zijlstra (Intel),
	Ingo Molnar, Borislav Petkov, H. Peter Anvin, Dave Martin,
	Ard Biesheuvel

On 22/07/21 19:32, Mark Brown wrote:
> On Thu, Jul 22, 2021 at 06:51:57PM +0100, Valentin Schneider wrote:
>> Running v5.13-rt1 on my arm64 Juno board triggers:
>
> Acked-by: Mark Brown <broonie@kernel.org>
>

Thanks!

>> [   11.337654] WARNING: CPU: 4 PID: 1 at arch/arm64/kernel/fpsimd.c:296 task_fpsimd_load (arch/arm64/kernel/fpsimd.c:296 (discriminator 1))
>> [   11.337692] Modules linked in:
>> [   11.337705] CPU: 4 PID: 1 Comm: init Not tainted 5.13.0-rt1 #52
>> [   11.337719] Hardware name: ARM Juno development board (r0) (DT)
>> [   11.337727] pstate: 40000005 (nZcv daif -PAN -UAO -TCO BTYPE=--)
>> [   11.337740] pc : task_fpsimd_load (arch/arm64/kernel/fpsimd.c:296 (discriminator 1))
>> [   11.337755] lr : task_fpsimd_load (arch/arm64/kernel/fpsimd.c:296 (discriminator 3))
>> [   11.337769] sp : ffff800012f4bdd0
>> [   11.337775] x29: ffff800012f4bdd0 x28: ffff000800160000 x27: 0000000000000000
>> [   11.337797] x26: 0000000000000000 x25: 0000000000000000 x24: ffff0008001606f0
>> [   11.337816] x23: ffff000800160000 x22: ffff000800160700 x21: ffff000800160000
>
> For readability it's probably better to not include the entire register
> contents here - the backtrace ends up about as large as the actual
> changelog, a good chunk of which is just the register contents.

Noted.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/3] sched, x86, arm64: PREEMPT_RT, FPU and preemption
  2021-07-22 17:51 ` Valentin Schneider
@ 2021-07-23 14:12   ` Ard Biesheuvel
  -1 siblings, 0 replies; 16+ messages in thread
From: Ard Biesheuvel @ 2021-07-23 14:12 UTC (permalink / raw)
  To: Valentin Schneider
  Cc: Linux Kernel Mailing List, Linux ARM, linux-rt-users, X86 ML,
	Catalin Marinas, Will Deacon, Thomas Gleixner, Steven Rostedt,
	Daniel Bristot de Oliveira, Peter Zijlstra (Intel),
	Ingo Molnar, Borislav Petkov, H. Peter Anvin, Mark Brown,
	Dave Martin

On Thu, 22 Jul 2021 at 19:53, Valentin Schneider
<valentin.schneider@arm.com> wrote:
>
> Hi folks,
>
> This stems from some more v5.13-rt1 breakage on arm64. This time per-CPU access
> safety isn't sufficient, we really need to keep preemption disabled.
>
> In a happy accident I stumbled on
>
>   cba08c5dc6dc ("x86/fpu: Make kernel FPU protection RT friendly")
>
> so I packaged what was done there into some common helpers and plastered them
> over the problematic areas.
>
> Cheers,
> Valentin
>
> Valentin Schneider (3):
>   sched/preempt: Introduce preempt_{enable, disable}_bh()
>   x86/fpu: Make FPU protection reuse common helper
>   arm64/fpsimd: Fix FPSIMD context handling vs PREEMPT_RT
>

Thanks for fixing this.

Reviewed-by: Ard Biesheuvel <ardb@kernel.org>


>  arch/arm64/kernel/fpsimd.c     |  6 ++++--
>  arch/x86/include/asm/fpu/api.h | 19 ++-----------------
>  include/linux/bottom_half.h    | 26 ++++++++++++++++++++++++++
>  3 files changed, 32 insertions(+), 19 deletions(-)
>
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/3] sched, x86, arm64: PREEMPT_RT, FPU and preemption
@ 2021-07-23 14:12   ` Ard Biesheuvel
  0 siblings, 0 replies; 16+ messages in thread
From: Ard Biesheuvel @ 2021-07-23 14:12 UTC (permalink / raw)
  To: Valentin Schneider
  Cc: Linux Kernel Mailing List, Linux ARM, linux-rt-users, X86 ML,
	Catalin Marinas, Will Deacon, Thomas Gleixner, Steven Rostedt,
	Daniel Bristot de Oliveira, Peter Zijlstra (Intel),
	Ingo Molnar, Borislav Petkov, H. Peter Anvin, Mark Brown,
	Dave Martin

On Thu, 22 Jul 2021 at 19:53, Valentin Schneider
<valentin.schneider@arm.com> wrote:
>
> Hi folks,
>
> This stems from some more v5.13-rt1 breakage on arm64. This time per-CPU access
> safety isn't sufficient, we really need to keep preemption disabled.
>
> In a happy accident I stumbled on
>
>   cba08c5dc6dc ("x86/fpu: Make kernel FPU protection RT friendly")
>
> so I packaged what was done there into some common helpers and plastered them
> over the problematic areas.
>
> Cheers,
> Valentin
>
> Valentin Schneider (3):
>   sched/preempt: Introduce preempt_{enable, disable}_bh()
>   x86/fpu: Make FPU protection reuse common helper
>   arm64/fpsimd: Fix FPSIMD context handling vs PREEMPT_RT
>

Thanks for fixing this.

Reviewed-by: Ard Biesheuvel <ardb@kernel.org>


>  arch/arm64/kernel/fpsimd.c     |  6 ++++--
>  arch/x86/include/asm/fpu/api.h | 19 ++-----------------
>  include/linux/bottom_half.h    | 26 ++++++++++++++++++++++++++
>  3 files changed, 32 insertions(+), 19 deletions(-)
>
> --
> 2.25.1
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2021-07-23 14:14 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-22 17:51 [PATCH 0/3] sched, x86, arm64: PREEMPT_RT, FPU and preemption Valentin Schneider
2021-07-22 17:51 ` Valentin Schneider
2021-07-22 17:51 ` [PATCH 1/3] sched/preempt: Introduce preempt_{enable, disable}_bh() Valentin Schneider
2021-07-22 17:51   ` Valentin Schneider
2021-07-22 17:51 ` [PATCH 2/3] x86/fpu: Make FPU protection reuse common helper Valentin Schneider
2021-07-22 17:51   ` Valentin Schneider
2021-07-22 17:51 ` [PATCH 3/3] arm64/fpsimd: Fix FPSIMD context handling vs PREEMPT_RT Valentin Schneider
2021-07-22 17:51   ` Valentin Schneider
2021-07-22 18:32   ` Mark Brown
2021-07-22 18:32     ` Mark Brown
2021-07-23 11:07     ` Valentin Schneider
2021-07-23 11:07       ` Valentin Schneider
2021-07-22 20:11 ` [PATCH 0/3] sched, x86, arm64: PREEMPT_RT, FPU and preemption Andrew Halaney
2021-07-22 20:11   ` Andrew Halaney
2021-07-23 14:12 ` Ard Biesheuvel
2021-07-23 14:12   ` Ard Biesheuvel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.