* [PATCH 1/3] sched/preempt: Introduce preempt_{enable, disable}_bh()
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 2/3] x86/fpu: Make FPU protection reuse common helper Valentin Schneider
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ 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] 8+ messages in thread* [PATCH 2/3] x86/fpu: Make FPU protection reuse common helper
2021-07-22 17:51 [PATCH 0/3] sched, x86, arm64: PREEMPT_RT, FPU and preemption 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 3/3] arm64/fpsimd: Fix FPSIMD context handling vs PREEMPT_RT Valentin Schneider
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ 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] 8+ messages in thread* [PATCH 3/3] arm64/fpsimd: Fix FPSIMD context handling vs PREEMPT_RT
2021-07-22 17:51 [PATCH 0/3] sched, x86, arm64: PREEMPT_RT, FPU and preemption Valentin Schneider
2021-07-22 17:51 ` [PATCH 1/3] sched/preempt: Introduce preempt_{enable, disable}_bh() 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 18:32 ` Mark Brown
2021-07-22 20:11 ` [PATCH 0/3] sched, x86, arm64: PREEMPT_RT, FPU and preemption Andrew Halaney
2021-07-23 14:12 ` Ard Biesheuvel
4 siblings, 1 reply; 8+ 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] 8+ messages in thread* Re: [PATCH 3/3] arm64/fpsimd: Fix FPSIMD context handling vs PREEMPT_RT
2021-07-22 17:51 ` [PATCH 3/3] arm64/fpsimd: Fix FPSIMD context handling vs PREEMPT_RT Valentin Schneider
@ 2021-07-22 18:32 ` Mark Brown
2021-07-23 11:07 ` Valentin Schneider
0 siblings, 1 reply; 8+ 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] 8+ 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
0 siblings, 0 replies; 8+ 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] 8+ messages in thread
* Re: [PATCH 0/3] sched, x86, arm64: PREEMPT_RT, FPU and preemption
2021-07-22 17:51 [PATCH 0/3] sched, x86, arm64: PREEMPT_RT, FPU and preemption Valentin Schneider
` (2 preceding siblings ...)
2021-07-22 17:51 ` [PATCH 3/3] arm64/fpsimd: Fix FPSIMD context handling vs PREEMPT_RT Valentin Schneider
@ 2021-07-22 20:11 ` Andrew Halaney
2021-07-23 14:12 ` Ard Biesheuvel
4 siblings, 0 replies; 8+ 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] 8+ messages in thread* Re: [PATCH 0/3] sched, x86, arm64: PREEMPT_RT, FPU and preemption
2021-07-22 17:51 [PATCH 0/3] sched, x86, arm64: PREEMPT_RT, FPU and preemption Valentin Schneider
` (3 preceding siblings ...)
2021-07-22 20:11 ` [PATCH 0/3] sched, x86, arm64: PREEMPT_RT, FPU and preemption Andrew Halaney
@ 2021-07-23 14:12 ` Ard Biesheuvel
4 siblings, 0 replies; 8+ 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] 8+ messages in thread