linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] arm64: arch_timer: Ensure counter register reads occur with seqlock held
@ 2019-04-29 16:59 Will Deacon
  2019-04-30  9:42 ` Vincenzo Frascino
  2019-04-30 10:32 ` Sasha Levin
  0 siblings, 2 replies; 3+ messages in thread
From: Will Deacon @ 2019-04-29 16:59 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Marc Zyngier, tglx, Vincenzo Frascino, Will Deacon, stable

When executing clock_gettime(), either in the vDSO or via a system call,
we need to ensure that the read of the counter register occurs within
the seqlock reader critical section. This ensures that updates to the
clocksource parameters (e.g. the multiplier) are consistent with the
counter value and therefore avoids the situation where time appears to
go backwards across multiple reads.

Extend the vDSO logic so that the seqlock critical section covers the
read of the counter register as well as accesses to the data page. Since
reads of the counter system registers are not ordered by memory barrier
instructions, introduce dependency ordering from the counter read to a
subsequent memory access so that the seqlock memory barriers apply to
the counter access in both the vDSO and the system call paths.

Cc: <stable@vger.kernel.org>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Vincenzo Frascino <vincenzo.frascino@arm.com>
Link: https://lore.kernel.org/linux-arm-kernel/alpine.DEB.2.21.1902081950260.1662@nanos.tec.linutronix.de/
Reported-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/asm/arch_timer.h   | 33 +++++++++++++++++++++++++++++++--
 arch/arm64/kernel/vdso/gettimeofday.S | 15 +++++++++++----
 2 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index f2a234d6516c..93e07512b4b6 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -148,18 +148,47 @@ static inline void arch_timer_set_cntkctl(u32 cntkctl)
 	isb();
 }
 
+/*
+ * Ensure that reads of the counter are treated the same as memory reads
+ * for the purposes of ordering by subsequent memory barriers.
+ *
+ * This insanity brought to you by speculative system register reads,
+ * out-of-order memory accesses, sequence locks and Thomas Gleixner.
+ *
+ * http://lists.infradead.org/pipermail/linux-arm-kernel/2019-February/631195.html
+ */
+#define arch_counter_enforce_ordering(val) do {				\
+	u64 tmp, _val = (val);						\
+									\
+	asm volatile(							\
+	"	eor	%0, %1, %1\n"					\
+	"	add	%0, sp, %0\n"					\
+	"	ldr	xzr, [%0]"					\
+	: "=r" (tmp) : "r" (_val));					\
+} while (0)
+
 static inline u64 arch_counter_get_cntpct(void)
 {
+	u64 cnt;
+
 	isb();
-	return arch_timer_reg_read_stable(cntpct_el0);
+	cnt = arch_timer_reg_read_stable(cntpct_el0);
+	arch_counter_enforce_ordering(cnt);
+	return cnt;
 }
 
 static inline u64 arch_counter_get_cntvct(void)
 {
+	u64 cnt;
+
 	isb();
-	return arch_timer_reg_read_stable(cntvct_el0);
+	cnt = arch_timer_reg_read_stable(cntvct_el0);
+	arch_counter_enforce_ordering(cnt);
+	return cnt;
 }
 
+#undef arch_counter_enforce_ordering
+
 static inline int arch_timer_arch_init(void)
 {
 	return 0;
diff --git a/arch/arm64/kernel/vdso/gettimeofday.S b/arch/arm64/kernel/vdso/gettimeofday.S
index 21805e416483..856fee6d3512 100644
--- a/arch/arm64/kernel/vdso/gettimeofday.S
+++ b/arch/arm64/kernel/vdso/gettimeofday.S
@@ -73,6 +73,13 @@ x_tmp		.req	x8
 	movn	x_tmp, #0xff00, lsl #48
 	and	\res, x_tmp, \res
 	mul	\res, \res, \mult
+	/*
+	 * Fake address dependency from the value computed from the counter
+	 * register to subsequent data page accesses so that the sequence
+	 * locking also orders the read of the counter.
+	 */
+	and	x_tmp, \res, xzr
+	add	vdso_data, vdso_data, x_tmp
 	.endm
 
 	/*
@@ -147,12 +154,12 @@ ENTRY(__kernel_gettimeofday)
 	/* w11 = cs_mono_mult, w12 = cs_shift */
 	ldp	w11, w12, [vdso_data, #VDSO_CS_MONO_MULT]
 	ldp	x13, x14, [vdso_data, #VDSO_XTIME_CLK_SEC]
-	seqcnt_check fail=1b
 
 	get_nsec_per_sec res=x9
 	lsl	x9, x9, x12
 
 	get_clock_shifted_nsec res=x15, cycle_last=x10, mult=x11
+	seqcnt_check fail=1b
 	get_ts_realtime res_sec=x10, res_nsec=x11, \
 		clock_nsec=x15, xtime_sec=x13, xtime_nsec=x14, nsec_to_sec=x9
 
@@ -211,13 +218,13 @@ realtime:
 	/* w11 = cs_mono_mult, w12 = cs_shift */
 	ldp	w11, w12, [vdso_data, #VDSO_CS_MONO_MULT]
 	ldp	x13, x14, [vdso_data, #VDSO_XTIME_CLK_SEC]
-	seqcnt_check fail=realtime
 
 	/* All computations are done with left-shifted nsecs. */
 	get_nsec_per_sec res=x9
 	lsl	x9, x9, x12
 
 	get_clock_shifted_nsec res=x15, cycle_last=x10, mult=x11
+	seqcnt_check fail=realtime
 	get_ts_realtime res_sec=x10, res_nsec=x11, \
 		clock_nsec=x15, xtime_sec=x13, xtime_nsec=x14, nsec_to_sec=x9
 	clock_gettime_return, shift=1
@@ -231,7 +238,6 @@ monotonic:
 	ldp	w11, w12, [vdso_data, #VDSO_CS_MONO_MULT]
 	ldp	x13, x14, [vdso_data, #VDSO_XTIME_CLK_SEC]
 	ldp	x3, x4, [vdso_data, #VDSO_WTM_CLK_SEC]
-	seqcnt_check fail=monotonic
 
 	/* All computations are done with left-shifted nsecs. */
 	lsl	x4, x4, x12
@@ -239,6 +245,7 @@ monotonic:
 	lsl	x9, x9, x12
 
 	get_clock_shifted_nsec res=x15, cycle_last=x10, mult=x11
+	seqcnt_check fail=monotonic
 	get_ts_realtime res_sec=x10, res_nsec=x11, \
 		clock_nsec=x15, xtime_sec=x13, xtime_nsec=x14, nsec_to_sec=x9
 
@@ -253,13 +260,13 @@ monotonic_raw:
 	/* w11 = cs_raw_mult, w12 = cs_shift */
 	ldp	w12, w11, [vdso_data, #VDSO_CS_SHIFT]
 	ldp	x13, x14, [vdso_data, #VDSO_RAW_TIME_SEC]
-	seqcnt_check fail=monotonic_raw
 
 	/* All computations are done with left-shifted nsecs. */
 	get_nsec_per_sec res=x9
 	lsl	x9, x9, x12
 
 	get_clock_shifted_nsec res=x15, cycle_last=x10, mult=x11
+	seqcnt_check fail=monotonic_raw
 	get_ts_clock_raw res_sec=x10, res_nsec=x11, \
 		clock_nsec=x15, nsec_to_sec=x9
 
-- 
2.11.0


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

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] arm64: arch_timer: Ensure counter register reads occur with seqlock held
  2019-04-29 16:59 [PATCH] arm64: arch_timer: Ensure counter register reads occur with seqlock held Will Deacon
@ 2019-04-30  9:42 ` Vincenzo Frascino
  2019-04-30 10:32 ` Sasha Levin
  1 sibling, 0 replies; 3+ messages in thread
From: Vincenzo Frascino @ 2019-04-30  9:42 UTC (permalink / raw)
  To: Will Deacon, linux-arm-kernel; +Cc: Marc Zyngier, tglx, stable

Hi Will,

On 29/04/2019 17:59, Will Deacon wrote:
> When executing clock_gettime(), either in the vDSO or via a system call,
> we need to ensure that the read of the counter register occurs within
> the seqlock reader critical section. This ensures that updates to the
> clocksource parameters (e.g. the multiplier) are consistent with the
> counter value and therefore avoids the situation where time appears to
> go backwards across multiple reads.
> 
> Extend the vDSO logic so that the seqlock critical section covers the
> read of the counter register as well as accesses to the data page. Since
> reads of the counter system registers are not ordered by memory barrier
> instructions, introduce dependency ordering from the counter read to a
> subsequent memory access so that the seqlock memory barriers apply to
> the counter access in both the vDSO and the system call paths.
> 

I tested it and seems ok to me, based on this:
Tested-by: Vincenzo Frascino <vincenzo.frascino@arm.com>

-- 
Regards,
Vincenzo

> Cc: <stable@vger.kernel.org>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Vincenzo Frascino <vincenzo.frascino@arm.com>
> Link: https://lore.kernel.org/linux-arm-kernel/alpine.DEB.2.21.1902081950260.1662@nanos.tec.linutronix.de/
> Reported-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm64/include/asm/arch_timer.h   | 33 +++++++++++++++++++++++++++++++--
>  arch/arm64/kernel/vdso/gettimeofday.S | 15 +++++++++++----
>  2 files changed, 42 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
> index f2a234d6516c..93e07512b4b6 100644
> --- a/arch/arm64/include/asm/arch_timer.h
> +++ b/arch/arm64/include/asm/arch_timer.h
> @@ -148,18 +148,47 @@ static inline void arch_timer_set_cntkctl(u32 cntkctl)
>  	isb();
>  }
>  
> +/*
> + * Ensure that reads of the counter are treated the same as memory reads
> + * for the purposes of ordering by subsequent memory barriers.
> + *
> + * This insanity brought to you by speculative system register reads,
> + * out-of-order memory accesses, sequence locks and Thomas Gleixner.
> + *
> + * http://lists.infradead.org/pipermail/linux-arm-kernel/2019-February/631195.html
> + */
> +#define arch_counter_enforce_ordering(val) do {				\
> +	u64 tmp, _val = (val);						\
> +									\
> +	asm volatile(							\
> +	"	eor	%0, %1, %1\n"					\
> +	"	add	%0, sp, %0\n"					\
> +	"	ldr	xzr, [%0]"					\
> +	: "=r" (tmp) : "r" (_val));					\
> +} while (0)
> +
>  static inline u64 arch_counter_get_cntpct(void)
>  {
> +	u64 cnt;
> +
>  	isb();
> -	return arch_timer_reg_read_stable(cntpct_el0);
> +	cnt = arch_timer_reg_read_stable(cntpct_el0);
> +	arch_counter_enforce_ordering(cnt);
> +	return cnt;
>  }
>  
>  static inline u64 arch_counter_get_cntvct(void)
>  {
> +	u64 cnt;
> +
>  	isb();
> -	return arch_timer_reg_read_stable(cntvct_el0);
> +	cnt = arch_timer_reg_read_stable(cntvct_el0);
> +	arch_counter_enforce_ordering(cnt);
> +	return cnt;
>  }
>  
> +#undef arch_counter_enforce_ordering
> +
>  static inline int arch_timer_arch_init(void)
>  {
>  	return 0;
> diff --git a/arch/arm64/kernel/vdso/gettimeofday.S b/arch/arm64/kernel/vdso/gettimeofday.S
> index 21805e416483..856fee6d3512 100644
> --- a/arch/arm64/kernel/vdso/gettimeofday.S
> +++ b/arch/arm64/kernel/vdso/gettimeofday.S
> @@ -73,6 +73,13 @@ x_tmp		.req	x8
>  	movn	x_tmp, #0xff00, lsl #48
>  	and	\res, x_tmp, \res
>  	mul	\res, \res, \mult
> +	/*
> +	 * Fake address dependency from the value computed from the counter
> +	 * register to subsequent data page accesses so that the sequence
> +	 * locking also orders the read of the counter.
> +	 */
> +	and	x_tmp, \res, xzr
> +	add	vdso_data, vdso_data, x_tmp
>  	.endm
>  
>  	/*
> @@ -147,12 +154,12 @@ ENTRY(__kernel_gettimeofday)
>  	/* w11 = cs_mono_mult, w12 = cs_shift */
>  	ldp	w11, w12, [vdso_data, #VDSO_CS_MONO_MULT]
>  	ldp	x13, x14, [vdso_data, #VDSO_XTIME_CLK_SEC]
> -	seqcnt_check fail=1b
>  
>  	get_nsec_per_sec res=x9
>  	lsl	x9, x9, x12
>  
>  	get_clock_shifted_nsec res=x15, cycle_last=x10, mult=x11
> +	seqcnt_check fail=1b
>  	get_ts_realtime res_sec=x10, res_nsec=x11, \
>  		clock_nsec=x15, xtime_sec=x13, xtime_nsec=x14, nsec_to_sec=x9
>  
> @@ -211,13 +218,13 @@ realtime:
>  	/* w11 = cs_mono_mult, w12 = cs_shift */
>  	ldp	w11, w12, [vdso_data, #VDSO_CS_MONO_MULT]
>  	ldp	x13, x14, [vdso_data, #VDSO_XTIME_CLK_SEC]
> -	seqcnt_check fail=realtime
>  
>  	/* All computations are done with left-shifted nsecs. */
>  	get_nsec_per_sec res=x9
>  	lsl	x9, x9, x12
>  
>  	get_clock_shifted_nsec res=x15, cycle_last=x10, mult=x11
> +	seqcnt_check fail=realtime
>  	get_ts_realtime res_sec=x10, res_nsec=x11, \
>  		clock_nsec=x15, xtime_sec=x13, xtime_nsec=x14, nsec_to_sec=x9
>  	clock_gettime_return, shift=1
> @@ -231,7 +238,6 @@ monotonic:
>  	ldp	w11, w12, [vdso_data, #VDSO_CS_MONO_MULT]
>  	ldp	x13, x14, [vdso_data, #VDSO_XTIME_CLK_SEC]
>  	ldp	x3, x4, [vdso_data, #VDSO_WTM_CLK_SEC]
> -	seqcnt_check fail=monotonic
>  
>  	/* All computations are done with left-shifted nsecs. */
>  	lsl	x4, x4, x12
> @@ -239,6 +245,7 @@ monotonic:
>  	lsl	x9, x9, x12
>  
>  	get_clock_shifted_nsec res=x15, cycle_last=x10, mult=x11
> +	seqcnt_check fail=monotonic
>  	get_ts_realtime res_sec=x10, res_nsec=x11, \
>  		clock_nsec=x15, xtime_sec=x13, xtime_nsec=x14, nsec_to_sec=x9
>  
> @@ -253,13 +260,13 @@ monotonic_raw:
>  	/* w11 = cs_raw_mult, w12 = cs_shift */
>  	ldp	w12, w11, [vdso_data, #VDSO_CS_SHIFT]
>  	ldp	x13, x14, [vdso_data, #VDSO_RAW_TIME_SEC]
> -	seqcnt_check fail=monotonic_raw
>  
>  	/* All computations are done with left-shifted nsecs. */
>  	get_nsec_per_sec res=x9
>  	lsl	x9, x9, x12
>  
>  	get_clock_shifted_nsec res=x15, cycle_last=x10, mult=x11
> +	seqcnt_check fail=monotonic_raw
>  	get_ts_clock_raw res_sec=x10, res_nsec=x11, \
>  		clock_nsec=x15, nsec_to_sec=x9
>  
> 


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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] arm64: arch_timer: Ensure counter register reads occur with seqlock held
  2019-04-29 16:59 [PATCH] arm64: arch_timer: Ensure counter register reads occur with seqlock held Will Deacon
  2019-04-30  9:42 ` Vincenzo Frascino
@ 2019-04-30 10:32 ` Sasha Levin
  1 sibling, 0 replies; 3+ messages in thread
From: Sasha Levin @ 2019-04-30 10:32 UTC (permalink / raw)
  To: Sasha Levin, Will Deacon, linux-arm-kernel
  Cc: Marc Zyngier, Will Deacon, stable, tglx, Vincenzo Frascino

Hi,

[This is an automated email]

This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all

The bot has tested the following trees: v5.0.10, v4.19.37, v4.14.114, v4.9.171, v4.4.179, v3.18.139.

v5.0.10: Build OK!
v4.19.37: Build OK!
v4.14.114: Failed to apply! Possible dependencies:
    f2e600c149fd ("arm64: Implement arch_counter_get_cntpct to read the physical counter")

v4.9.171: Failed to apply! Possible dependencies:
    16d10ef29f25 ("clocksource/drivers/arm_arch_timer: Introduce generic errata handling infrastructure")
    bb42ca474010 ("clocksource/drivers/arm_arch_timer: Work around Hisilicon erratum 161010101")
    d003d029cea8 ("arm64: arch_timer: Add HISILICON_ERRATUM_161010101 ACPI matching data")
    f2e600c149fd ("arm64: Implement arch_counter_get_cntpct to read the physical counter")
    fa8d815fac96 ("arm64: arch_timer: Workaround for Cortex-A73 erratum 858921")

v4.4.179: Failed to apply! Possible dependencies:
    0cb0786bac15 ("ARM64: PCI: Support ACPI-based PCI host controller")
    16d10ef29f25 ("clocksource/drivers/arm_arch_timer: Introduce generic errata handling infrastructure")
    1a4f93f7112f ("PCI: Factor DT-specific pci_bus_find_domain_nr() code out")
    1bd37a6835be ("iommu/arm-smmu: Workaround for ThunderX erratum #27704")
    1d8f51d41fc7 ("arm/arm64: arch_timer: Use archdata to indicate vdso suitability")
    21266be9ed54 ("arch: consolidate CONFIG_STRICT_DEVM in lib/Kconfig.debug")
    2ab51ddeca2f ("ARM64: PCI: Add acpi_pci_bus_find_domain_nr()")
    46fd5c6b3059 ("clocksource/drivers/arm_arch_timer: Control the evtstrm via the cmdline")
    4e3e9b6997b2 ("iommu/arm-smmu: Add support for 16 bit VMID")
    75df1386557c ("iommu/arm-smmu: Invalidate TLBs properly")
    9c7cb891ecfe ("PCI: Refactor pci_bus_assign_domain_nr() for CONFIG_PCI_DOMAINS_GENERIC")
    cd5f22d7967f ("arm64: arch_timer: simplify accessors")
    f2e600c149fd ("arm64: Implement arch_counter_get_cntpct to read the physical counter")
    f6dc1576cd51 ("arm64: arch_timer: Work around QorIQ Erratum A-008585")

v3.18.139: Failed to apply! Possible dependencies:
    04597a65c5ef ("arm64: Track system support for mixed endian EL0")
    104a0c02e8b1 ("arm64: Add workaround for Cavium erratum 27456")
    16d10ef29f25 ("clocksource/drivers/arm_arch_timer: Introduce generic errata handling infrastructure")
    1b907f46db07 ("arm64: kconfig: move emulation option under kernel features")
    1bd37a6835be ("iommu/arm-smmu: Workaround for ThunderX erratum #27704")
    2d888f48e056 ("arm64: Emulate SETEND for AArch32 tasks")
    338d4f49d6f7 ("arm64: kernel: Add support for Privileged Access Never")
    359b706473b4 ("arm64: Extract feature parsing code from cpu_errata.c")
    587064b610c7 ("arm64: Add framework for legacy instruction emulation")
    6d4e11c5e2e8 ("irqchip/gicv3: Workaround for Cavium ThunderX erratum 23154")
    736d474f0faf ("arm64: Consolidate hotplug notifier for instruction emulation")
    870828e57b14 ("arm64: kernel: Move config_sctlr_el1")
    94a9e04aa16a ("arm64: alternative: Introduce feature for GICv3 CPU interface")
    9b79f52d1a70 ("arm64: Add support for hooks to handle undefined instructions")
    9cb9c9e5ba84 ("arm64: Documentation: add list of software workarounds for errata")
    bd35a4adc413 ("arm64: Port SWP/SWPB emulation support from arm")
    c739dc83a0b6 ("arm64: lse: rename ARM64_CPU_FEAT_LSE_ATOMICS for consistency")
    c852f3205846 ("arm64: Emulate CP15 Barrier instructions")
    c9453a3ab1a3 ("arm64: alternatives: fix pr_fmt string for consistency")
    f2e600c149fd ("arm64: Implement arch_counter_get_cntpct to read the physical counter")
    f6dc1576cd51 ("arm64: arch_timer: Work around QorIQ Erratum A-008585")


How should we proceed with this patch?

--
Thanks,
Sasha

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-04-30 10:32 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-04-29 16:59 [PATCH] arm64: arch_timer: Ensure counter register reads occur with seqlock held Will Deacon
2019-04-30  9:42 ` Vincenzo Frascino
2019-04-30 10:32 ` Sasha Levin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).