Skip to contentSkip to conversation

arm64: add an MTE support check to the top of mte_thread_switch()

PATCH Superseded Peter Collingbourne 2 participants 4 messages Sep 14, 2021 – Sep 15, 2021

Subsystems (2)
arm64 KASAN

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Peter Collingbourne

author
#0 lore raw

PATCH v2 1/2 arm64: add an MTE support check to the top of mte_thread_switch()

This lets us avoid doing unnecessary work on hardware that does not support MTE, and will allow us to freely use MTE instructions in the code called by mte_thread_switch().

Signed-off-by Peter Collingbourne <[email protected]>
Link https://linux-review.googlesource.com/id/I02fd000d1ef2c86c7d2952a7f099b254ec227a5d
---
 arch/arm64/kernel/mte.c | 3 +++
 1 file changed, 3 insertions(+)
arch/arm64/kernel/mte.c +3 -0
index 9d314a3bad3b..8fc2d90eeb00 100644
@@ -199,6 +199,9 @@ void mte_thread_init_user(void)
 
 void mte_thread_switch(struct task_struct *next)
 {
+	if (!system_supports_mte())
+		return;
+
 	mte_update_sctlr_user(next);
 
 	/*
Signature · 7 lines
2.33.0.309.g3052b89438-goog


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
author
#1 lore raw

PATCH v2 2/2 arm64: kasan: mte: move GCR_EL1 switch to task switch when KASAN disabled

It is not necessary to write to GCR_EL1 on every kernel entry and exit when HW tag-based KASAN is disabled because the kernel will not execute any IRG instructions in that mode. Since accessing GCR_EL1 can be expensive on some microarchitectures, avoid doing so by moving the access to task switch when HW tag-based KASAN is disabled.

Signed-off-by Peter Collingbourne <[email protected]>
Acked-by Andrey Konovalov <[email protected]>
Link https://linux-review.googlesource.com/id/I78e90d60612a94c24344526f476ac4ff216e10d2

---
v2:
- add prerequisite patch to fix booting on non-MTE hardware

 arch/arm64/kernel/entry.S | 10 +++++-----
 arch/arm64/kernel/mte.c   | 24 +++++++++++++++++++++---
 include/linux/kasan.h     |  9 +++++++--
 3 files changed, 33 insertions(+), 10 deletions(-)
arch/arm64/kernel/entry.S +5 -5
index bc6d5a970a13..2f69ae43941d 100644
@@ -168,9 +168,9 @@ alternative_else_nop_endif
 
 	.macro mte_set_kernel_gcr, tmp, tmp2
 #ifdef CONFIG_KASAN_HW_TAGS
-alternative_if_not ARM64_MTE
+alternative_cb	kasan_hw_tags_enable
 	b	1f
-alternative_else_nop_endif
+alternative_cb_end
 	mov	\tmp, KERNEL_GCR_EL1
 	msr_s	SYS_GCR_EL1, \tmp
 1:

@@ -178,10 +178,10 @@ alternative_else_nop_endif
 	.endm
 
 	.macro mte_set_user_gcr, tsk, tmp, tmp2
-#ifdef CONFIG_ARM64_MTE
-alternative_if_not ARM64_MTE
+#ifdef CONFIG_KASAN_HW_TAGS
+alternative_cb	kasan_hw_tags_enable
 	b	1f
-alternative_else_nop_endif
+alternative_cb_end
 	ldr	\tmp, [\tsk, #THREAD_MTE_CTRL]
 
 	mte_set_gcr \tmp, \tmp2
arch/arm64/kernel/mte.c +21 -3
index 8fc2d90eeb00..5a026a91b4bf 100644
@@ -162,7 +162,7 @@ void mte_check_tfsr_el1(void)
 }
 #endif
 
-static void mte_update_sctlr_user(struct task_struct *task)
+static void mte_update_sctlr_user_and_gcr_excl(struct task_struct *task)
 {
 	/*
 	 * This must be called with preemption disabled and can only be called

@@ -182,6 +182,24 @@ static void mte_update_sctlr_user(struct task_struct *task)
 	else if (resolved_mte_tcf & MTE_CTRL_TCF_SYNC)
 		sctlr |= SCTLR_EL1_TCF0_SYNC;
 	task->thread.sctlr_user = sctlr;
+
+	/*
+	 * SYS_GCR_EL1 will be set to current->thread.mte_ctrl value by
+	 * mte_set_user_gcr() in kernel_exit, but only if KASAN is enabled.
+	 */
+	if (!kasan_hw_tags_enabled())
+		write_sysreg_s(((mte_ctrl >> MTE_CTRL_GCR_USER_EXCL_SHIFT) &
+				SYS_GCR_EL1_EXCL_MASK) | SYS_GCR_EL1_RRND,
+			       SYS_GCR_EL1);
+}
+
+void __init kasan_hw_tags_enable(struct alt_instr *alt, __le32 *origptr,
+				 __le32 *updptr, int nr_inst)
+{
+	BUG_ON(nr_inst != 1); /* Branch -> NOP */
+
+	if (kasan_hw_tags_enabled())
+		*updptr = cpu_to_le32(aarch64_insn_gen_nop());
 }
 
 void mte_thread_init_user(void)

@@ -202,7 +220,7 @@ void mte_thread_switch(struct task_struct *next)
 	if (!system_supports_mte())
 		return;
 
-	mte_update_sctlr_user(next);
+	mte_update_sctlr_user_and_gcr_excl(next);
 
 	/*
 	 * Check if an async tag exception occurred at EL1.

@@ -247,7 +265,7 @@ long set_mte_ctrl(struct task_struct *task, unsigned long arg)
 	task->thread.mte_ctrl = mte_ctrl;
 	if (task == current) {
 		preempt_disable();
-		mte_update_sctlr_user(task);
+		mte_update_sctlr_user_and_gcr_excl(task);
 		update_sctlr_el1(task->thread.sctlr_user);
 		preempt_enable();
 	}
include/linux/kasan.h +7 -2
index dd874a1ee862..de5f5913374d 100644
@@ -89,7 +89,7 @@ static __always_inline bool kasan_enabled(void)
 	return static_branch_likely(&kasan_flag_enabled);
 }
 
-static inline bool kasan_has_integrated_init(void)
+static inline bool kasan_hw_tags_enabled(void)
 {
 	return kasan_enabled();
 }

@@ -104,7 +104,7 @@ static inline bool kasan_enabled(void)
 	return IS_ENABLED(CONFIG_KASAN);
 }
 
-static inline bool kasan_has_integrated_init(void)
+static inline bool kasan_hw_tags_enabled(void)
 {
 	return false;
 }

@@ -125,6 +125,11 @@ static __always_inline void kasan_free_pages(struct page *page,
 
 #endif /* CONFIG_KASAN_HW_TAGS */
 
+static inline bool kasan_has_integrated_init(void)
+{
+	return kasan_hw_tags_enabled();
+}
+
 #ifdef CONFIG_KASAN
 
 struct kasan_cache {
Signature · 7 lines
2.33.0.309.g3052b89438-goog


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
maintainer
#2 lore raw
In reply to Peter Collingbourne
Quoted 15 lines · Peter Collingbourne

This lets us avoid doing unnecessary work on hardware that does not support MTE, and will allow us to freely use MTE instructions in the code called by mte_thread_switch().

Signed-off-by Peter Collingbourne <[email protected]>
Link https://linux-review.googlesource.com/id/I02fd000d1ef2c86c7d2952a7f099b254ec227a5d
---
 arch/arm64/kernel/mte.c | 3 +++
 1 file changed, 3 insertions(+)
arch/arm64/kernel/mte.c +0 -0
index 9d314a3bad3b..8fc2d90eeb00 100644

arch/arm64/kernel/mte.c void mte_thread_init_user(void) · line 205

Quoted 7 lines
arch/arm64/kernel/mte.c +3 -0
@@ -199,6 +199,9 @@ void mte_thread_init_user(void)
 
 void mte_thread_switch(struct task_struct *next)
 {
+	if (!system_supports_mte())
+		return;
+
 	mte_update_sctlr_user(next);

Ah, good point. While you are at this, could you please remove the system_supports_mte() check from mte_check_tfsr_el1() and add it to mte_check_tfsr_entry/exit? On the exit path, we unnecessarily do a dsb+isb even when MTE is not present.

I guess we could also skip the tfsr_el1 check if !system_uses_mte_async_mode() (well, I'd need to page in which TFSR bits are set for uaccess).

Thanks.

Signature · 6 lines
Catalin

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
author
#3 lore raw
Quoted 18 lines · Catalin Marinas
Quoted 16 lines · Peter Collingbourne

This lets us avoid doing unnecessary work on hardware that does not support MTE, and will allow us to freely use MTE instructions in the code called by mte_thread_switch().

Signed-off-by Peter Collingbourne <[email protected]>
Link https://linux-review.googlesource.com/id/I02fd000d1ef2c86c7d2952a7f099b254ec227a5d
---
 arch/arm64/kernel/mte.c | 3 +++
 1 file changed, 3 insertions(+)
arch/arm64/kernel/mte.c +0 -0
index 9d314a3bad3b..8fc2d90eeb00 100644
@@ -199,6 +199,9 @@ void mte_thread_init_user(void)
Quoted 11 lines

arch/arm64/kernel/mte.c void mte_thread_init_user(void) · line 205

Quoted 6 lines
arch/arm64/kernel/mte.c +3 -0
 void mte_thread_switch(struct task_struct *next)
 {
+     if (!system_supports_mte())
+             return;
+
      mte_update_sctlr_user(next);

Ah, good point. While you are at this, could you please remove the system_supports_mte() check from mte_check_tfsr_el1() and add it to mte_check_tfsr_entry/exit? On the exit path, we unnecessarily do a dsb+isb even when MTE is not present.

Thanks for the suggestion. Done in v3.

Peter

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel