[PATCH 1/2] arm64/gettimeofday: correct the note about isb in __arch_get_hw_counter()

Pingfan Liu kernelfans at gmail.com
Tue Mar 30 11:57:18 BST 2021


The seq lock in vdso_read_retry() is behind smp_rmb(), and can not be
speculated before the counter value due to the read dependency. Hence
the original note is misleading.

The description of getting counter value is not very clear. [1]
'mrs Xt, cntpct' may execute out of program order, either forward or
backward.

Hence this isb is still required to protect against backward speculation.
Correct the note around the code to show the motivation.

[1]: AArch64 Programmer's Guides Generic Timer:  3.1. Count and frequency
Signed-off-by: Pingfan Liu <kernelfans at gmail.com>
Cc: Catalin Marinas <catalin.marinas at arm.com>
Cc: Will Deacon <will at kernel.org>
Cc: Vincenzo Frascino <vincenzo.frascino at arm.com>
Cc: Thomas Gleixner <tglx at linutronix.de>
Cc: Mark Rutland <mark.rutland at arm.com>
Cc: Andrei Vagin <avagin at gmail.com>
Cc: Marc Zyngier <maz at kernel.org>
To: linux-arm-kernel at lists.infradead.org
---
 arch/arm64/include/asm/vdso/compat_gettimeofday.h | 9 ++++++---
 arch/arm64/include/asm/vdso/gettimeofday.h        | 9 ++++++---
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/include/asm/vdso/compat_gettimeofday.h b/arch/arm64/include/asm/vdso/compat_gettimeofday.h
index 7508b0ac1d21..b5dfda25a5dc 100644
--- a/arch/arm64/include/asm/vdso/compat_gettimeofday.h
+++ b/arch/arm64/include/asm/vdso/compat_gettimeofday.h
@@ -123,10 +123,13 @@ static __always_inline u64 __arch_get_hw_counter(s32 clock_mode,
 	isb();
 	asm volatile("mrrc p15, 1, %Q0, %R0, c14" : "=r" (res));
 	/*
-	 * This isb() is required to prevent that the seq lock is
-	 * speculated.
+	 * Getting count value may execute out of program order, either forward
+	 * or backward. Although the caller has a read dependency on @res, but
+	 * it can not protect backward speculation against no dependency
+	 * instruction. Beside this purpose, this isb also severs as a
+	 * compiler barrier for this __always_inline function.
 	 */
-	isb();
+	 isb();
 
 	return res;
 }
diff --git a/arch/arm64/include/asm/vdso/gettimeofday.h b/arch/arm64/include/asm/vdso/gettimeofday.h
index 631ab1281633..6988a730b878 100644
--- a/arch/arm64/include/asm/vdso/gettimeofday.h
+++ b/arch/arm64/include/asm/vdso/gettimeofday.h
@@ -84,10 +84,13 @@ static __always_inline u64 __arch_get_hw_counter(s32 clock_mode,
 	isb();
 	asm volatile("mrs %0, cntvct_el0" : "=r" (res) :: "memory");
 	/*
-	 * This isb() is required to prevent that the seq lock is
-	 * speculated.#
+	 * Getting count value may execute out of program order, either forward
+	 * or backward. Although the caller has a read dependency on @res, but
+	 * it can not protect backward speculation against no dependency
+	 * instruction. Beside this purpose, this isb also severs as a
+	 * compiler barrier for this __always_inline function.
 	 */
-	isb();
+	 isb();
 
 	return res;
 }
-- 
2.29.2




More information about the linux-arm-kernel mailing list