Skip to contentSkip to conversation

arm64: ssbs: Fix context-switch when SSBS instructions are present

PATCH In review Will Deacon 2 participants 4 messages Feb 6, 2020

Subsystems (2)
lore
Share & tools
lore

Will Deacon

author maintainer
#0 lore raw

PATCH 1/1 arm64: ssbs: Fix context-switch when SSBS instructions are present

When all CPUs in the system implement the SSBS instructions, we advertise this via an HWCAP and allow EL0 to toggle the SSBS field in PSTATE directly. Consequently, the state of the mitigation is not accurately tracked by the TIF_SSBD thread flag and the PSTATE value is authoritative.

Avoid forcing the SSBS field in context-switch on such a system, and simply rely on the PSTATE register instead.

Cc Marc Zyngier <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Srinivas Ramana <[email protected]>
Fixes cbdf8a189a66 ("arm64: Force SSBS on context switch")
---
 arch/arm64/kernel/process.c | 7 +++++++
 1 file changed, 7 insertions(+)
arch/arm64/kernel/process.c +7 -0
index d54586d5b031..45e867f40a7a 100644
@@ -466,6 +466,13 @@ static void ssbs_thread_switch(struct task_struct *next)
 	if (unlikely(next->flags & PF_KTHREAD))
 		return;
 
+	/*
+	 * If all CPUs implement the SSBS instructions, then we just
+	 * need to context-switch the PSTATE field.
+	 */
+	if (cpu_have_feature(cpu_feature(SSBS)))
+		return;
+
 	/* If the mitigation is enabled, then we leave SSBS clear. */
 	if ((arm64_get_ssbd_state() == ARM64_SSBD_FORCE_ENABLE) ||
 	    test_tsk_thread_flag(next, TIF_SSBD))
Signature · 1 line
2.25.0.341.g760bfbb309-goog
Reviewed-by on v1 1/1
#1 lore raw
Quoted 37 lines · Will Deacon

When all CPUs in the system implement the SSBS instructions, we advertise this via an HWCAP and allow EL0 to toggle the SSBS field in PSTATE directly. Consequently, the state of the mitigation is not accurately tracked by the TIF_SSBD thread flag and the PSTATE value is authoritative.

Avoid forcing the SSBS field in context-switch on such a system, and simply rely on the PSTATE register instead.

Cc Marc Zyngier <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Srinivas Ramana <[email protected]>
Fixes cbdf8a189a66 ("arm64: Force SSBS on context switch")
---
 arch/arm64/kernel/process.c | 7 +++++++
 1 file changed, 7 insertions(+)
arch/arm64/kernel/process.c +7 -0
index d54586d5b031..45e867f40a7a 100644
@@ -466,6 +466,13 @@ static void ssbs_thread_switch(struct task_struct 
 *next)
 	if (unlikely(next->flags & PF_KTHREAD))
 		return;
 
+	/*
+	 * If all CPUs implement the SSBS instructions, then we just
+	 * need to context-switch the PSTATE field.
+	 */
+	if (cpu_have_feature(cpu_feature(SSBS)))
+		return;
+
 	/* If the mitigation is enabled, then we leave SSBS clear. */
 	if ((arm64_get_ssbd_state() == ARM64_SSBD_FORCE_ENABLE) ||
Quoted 1 line

test_tsk_thread_flag(next, TIF_SSBD))

Looks goot to me.

        M.
Signature · 1 line
Jazz is not dead. It just smells funny...
author maintainer
#2 lore raw
Quoted 29 lines · Marc Zyngier
Quoted 28 lines · Will Deacon

When all CPUs in the system implement the SSBS instructions, we advertise this via an HWCAP and allow EL0 to toggle the SSBS field in PSTATE directly. Consequently, the state of the mitigation is not accurately tracked by the TIF_SSBD thread flag and the PSTATE value is authoritative.

Avoid forcing the SSBS field in context-switch on such a system, and simply rely on the PSTATE register instead.

Cc Marc Zyngier <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Srinivas Ramana <[email protected]>
Fixes cbdf8a189a66 ("arm64: Force SSBS on context switch")
---
 arch/arm64/kernel/process.c | 7 +++++++
 1 file changed, 7 insertions(+)
arch/arm64/kernel/process.c +0 -0
index d54586d5b031..45e867f40a7a 100644
@@ -466,6 +466,13 @@ static void ssbs_thread_switch(struct task_struct
 *next)
 	if (unlikely(next->flags & PF_KTHREAD))
 		return;
 
Quoted 12 lines
Quoted 10 lines
arch/arm64/kernel/process.c +7 -0
+	/*
+	 * If all CPUs implement the SSBS instructions, then we just
+	 * need to context-switch the PSTATE field.
+	 */
+	if (cpu_have_feature(cpu_feature(SSBS)))
+		return;
+
 	/* If the mitigation is enabled, then we leave SSBS clear. */
 	if ((arm64_get_ssbd_state() == ARM64_SSBD_FORCE_ENABLE) ||

test_tsk_thread_flag(next, TIF_SSBD))

Looks goot to me.

Ja!

Quoted 1 line

Cheers. It occurs to me that, although the patch is correct, the comment and the commit message need tweaking because we're actually predicating this on the presence of SSBS in any form, so the instructions may not be implemented. That's fine because the prctl() updates pstate, so it remains authoritative and can't be lost by one of the CPUs treating it as RAZ/WI.

I'll spin a v2 later on.

Will

#3 lore raw
Quoted 43 lines · Will Deacon
Quoted 29 lines · Marc Zyngier
Quoted 28 lines · Will Deacon

When all CPUs in the system implement the SSBS instructions, we advertise this via an HWCAP and allow EL0 to toggle the SSBS field in PSTATE directly. Consequently, the state of the mitigation is not accurately tracked by the TIF_SSBD thread flag and the PSTATE value is authoritative.

Avoid forcing the SSBS field in context-switch on such a system, and simply rely on the PSTATE register instead.

Cc Marc Zyngier <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Srinivas Ramana <[email protected]>
Fixes cbdf8a189a66 ("arm64: Force SSBS on context switch")
---
 arch/arm64/kernel/process.c | 7 +++++++
 1 file changed, 7 insertions(+)
arch/arm64/kernel/process.c +0 -0
index d54586d5b031..45e867f40a7a 100644
@@ -466,6 +466,13 @@ static void ssbs_thread_switch(struct task_struct
 *next)
 	if (unlikely(next->flags & PF_KTHREAD))
 		return;
 
Quoted 12 lines
Quoted 10 lines
arch/arm64/kernel/process.c +7 -0
+	/*
+	 * If all CPUs implement the SSBS instructions, then we just
+	 * need to context-switch the PSTATE field.
+	 */
+	if (cpu_have_feature(cpu_feature(SSBS)))
+		return;
+
 	/* If the mitigation is enabled, then we leave SSBS clear. */
 	if ((arm64_get_ssbd_state() == ARM64_SSBD_FORCE_ENABLE) ||

test_tsk_thread_flag(next, TIF_SSBD))

Looks goot to me.

Quoted 1 line

Ja!

Ach...

Quoted 8 lines
Quoted 2 lines

Cheers. It occurs to me that, although the patch is correct, the comment and the commit message need tweaking because we're actually predicating this on the presence of SSBS in any form, so the instructions may not be implemented. That's fine because the prctl() updates pstate, so it remains authoritative and can't be lost by one of the CPUs treating it as RAZ/WI.

True. It is the PSTATE bit that actually matters, not the presence of the control instruction.

Quoted 1 line

I'll spin a v2 later on.

Thanks,

        M.
Signature · 1 line
Jazz is not dead. It just smells funny...