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

[Pingfan Liu <kernelfans@gmail.com>]

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@gmail.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Vincenzo Frascino <vincenzo.frascino@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Andrei Vagin <avagin@gmail.com>
Cc: Marc Zyngier <maz@kernel.org>
To: linux-arm-kernel@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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel