Message ID | 20200131163728.5228-5-alexandru.elisei@arm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | arm/arm64: Various fixes | expand |
diff --git a/arm/timer.c b/arm/timer.c index c6ea108cfa4b..e758e84855c3 100644 --- a/arm/timer.c +++ b/arm/timer.c @@ -30,6 +30,7 @@ static void ptimer_unsupported_handler(struct pt_regs *regs, unsigned int esr) static u64 read_vtimer_counter(void) { + isb(); return read_sysreg(cntvct_el0); } @@ -68,6 +69,7 @@ static void write_vtimer_ctl(u64 val) static u64 read_ptimer_counter(void) { + isb(); return read_sysreg(cntpct_el0); }
Reads of the physical counter and the virtual counter registers "can occur speculatively and out of order relative to other instructions executed on the same PE" [1, 2]. There is no theoretical limit to the number of instructions that the CPU can reorder and we use the counter value to program the timer to fire in the future. Add an ISB before reading the counter to make sure the read instruction is not reordered too long in the past with regard to the instruction that programs the timer alarm, thus causing the timer to fire unexpectedly. This matches what Linux does (see arch/arm64/include/asm/arch_timer.h). Because we use the counter value to program the timer, we create a register dependency [3] between the value that we read and the value that we write to CVAL and thus we don't need a barrier after the read. Linux does things differently because the read needs to be ordered with regard to a memory load (more information in commit 75a19a0202db ("arm64: arch_timer: Ensure counter register reads occur with seqlock held")). This also matches what we already do in get_cntvct from lib/arm{,64}/asm/processor.h. [1] ARM DDI 0487E.a, section D11.2.1 [2] ARM DDI 0487E.a, section D11.2.2 [3] ARM DDI 0486E.a, section B2.3.2 Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> --- arm/timer.c | 2 ++ 1 file changed, 2 insertions(+)