All of lore.kernel.org
 help / color / mirror / Atom feed
From: Valentin Schneider <valentin.schneider@arm.com>
To: linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-rt-users@vger.kernel.org, x86@kernel.org
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Steven Rostedt <rostedt@goodmis.org>,
	Daniel Bristot de Oliveira <bristot@redhat.com>,
	"Peter Zijlstra (Intel)" <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>, Borislav Petkov <bp@alien8.de>,
	"H. Peter Anvin" <hpa@zytor.com>, Mark Brown <broonie@kernel.org>,
	Dave Martin <Dave.Martin@arm.com>,
	Ard Biesheuvel <ardb@kernel.org>
Subject: [PATCH 3/3] arm64/fpsimd: Fix FPSIMD context handling vs PREEMPT_RT
Date: Thu, 22 Jul 2021 18:51:57 +0100	[thread overview]
Message-ID: <20210722175157.1367122-4-valentin.schneider@arm.com> (raw)
In-Reply-To: <20210722175157.1367122-1-valentin.schneider@arm.com>

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


WARNING: multiple messages have this Message-ID (diff)
From: Valentin Schneider <valentin.schneider@arm.com>
To: linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-rt-users@vger.kernel.org, x86@kernel.org
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Steven Rostedt <rostedt@goodmis.org>,
	Daniel Bristot de Oliveira <bristot@redhat.com>,
	"Peter Zijlstra (Intel)" <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>, Borislav Petkov <bp@alien8.de>,
	"H. Peter Anvin" <hpa@zytor.com>, Mark Brown <broonie@kernel.org>,
	Dave Martin <Dave.Martin@arm.com>,
	Ard Biesheuvel <ardb@kernel.org>
Subject: [PATCH 3/3] arm64/fpsimd: Fix FPSIMD context handling vs PREEMPT_RT
Date: Thu, 22 Jul 2021 18:51:57 +0100	[thread overview]
Message-ID: <20210722175157.1367122-4-valentin.schneider@arm.com> (raw)
In-Reply-To: <20210722175157.1367122-1-valentin.schneider@arm.com>

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

  parent reply	other threads:[~2021-07-22 17:53 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 ` Valentin Schneider [this message]
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-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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210722175157.1367122-4-valentin.schneider@arm.com \
    --to=valentin.schneider@arm.com \
    --cc=Dave.Martin@arm.com \
    --cc=ardb@kernel.org \
    --cc=bp@alien8.de \
    --cc=bristot@redhat.com \
    --cc=broonie@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=hpa@zytor.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rt-users@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=will@kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.