Thread (8 messages) 8 messages, 4 authors, 2020-08-01

Re: [PATCH v2 1/2] arm64: Move handling of erratum 1418040 into C code

From: Will Deacon <will@kernel.org>
Date: 2020-07-31 10:43:17

On Fri, Jul 31, 2020 at 09:33:57AM +0100, Marc Zyngier wrote:
Instead of dealing with erratum 1418040 on each entry and exit,
let's move the handling to __switch_to() instead, which has
several advantages:

- It can be applied when it matters (switching between 32 and 64
  bit tasks).
- It is written in C (yay!)
- It can rely on static keys rather than alternatives

Tested-by: Sai Prakash Ranjan <redacted>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kernel/entry.S   | 21 ---------------------
 arch/arm64/kernel/process.c | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 21 deletions(-)
[...]
quoted hunk ↗ jump to hunk
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 6089638c7d43..8bbf066224ab 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -515,6 +515,40 @@ static void entry_task_switch(struct task_struct *next)
 	__this_cpu_write(__entry_task, next);
 }
 
+/*
+ * ARM erratum 1418040 handling, affecting the 32bit view of CNTVCT.
+ * Assuming the virtual counter is enabled at the beginning of times:
+ *
+ * - disable access when switching from a 64bit task to a 32bit task
+ * - enable access when switching from a 32bit task to a 64bit task
+ */
+static __always_inline
Do we need the __always_inline? None of the other calls from __switch_to()
have it.
+void erratum_1418040_thread_switch(struct task_struct *prev,
+				   struct task_struct *next)
+{
+	bool prev32, next32;
+	u64 val;
+
+	if (!(IS_ENABLED(CONFIG_ARM64_ERRATUM_1418040) &&
+	      cpus_have_const_cap(ARM64_WORKAROUND_1418040)))
+		return;
+
+	prev32 = is_compat_thread(task_thread_info(prev));
+	next32 = is_compat_thread(task_thread_info(next));
+
+	if (prev32 == next32)
+		return;
+
+	val = read_sysreg(cntkctl_el1);
+
+	if (prev32 & !next32)
I know they're bools but this is perverse!

Why can't it just be:

	if (next32)
		val &= ~ARCH_TIMER_USR_VCT_ACCESS_EN;
	else
		val |= ARCH_TIMER_USR_VCT_ACCESS_EN;

?

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help