[PATCH v3 0/8] arm64: arch_timer: Improve errata handling

[PATCH v3 0/8] arm64: arch_timer: Improve errata handling

[Marc Zyngier]

This is the third version of this series addressing a couple of
embarrassing bugs in the prolific arm64 timer errata handling
department. See below for the changes since v2.

Time is hard. Timers are harder. As a consequence, we have plenty of
broken counter/timer implementations in the wild, and an
infrastructure to deal with them.

However, what we have today suffers from a number of issues:

- if, on an heterogeneous system, affected CPUs are secondaries, we do
  record their broken state but don't correct anything

- we always play games with preemption in order to access per-CPU
  state, irrespective of the presence of broken CPUs

I hear someone saying "just use a static key to enable the errata and
be done with it". Good call, except that we need to do that from a
CPUHP callback, and that's deadlock central. We can't do it later,
because this could affect the CPU before the workaround is enabled.

However, not everything is lost if we turn the logic on its head:

- always start with the mitigations enabled, even if we don't know of
  any affected CPU

- once all CPUs have been seen once, and that we still haven't
  enabled any workaround, disable the mitigations globally.

With that, a normal kernel boot with all CPUs will quickly switch to
no mitigation on decent HW. If you're booting with CPUs disabled, this
will only kick in once you have booted them all.

Patches on top of 7.2-rc4.

* From v2 [1]:

  - Added a fix for the PMUv3 driver that really wants to know whether
    it is operating with the architected timer. This has been broken
    for some time (since 24ee7607b286b ("arm64/arch_timer: Provide
    noinstr sched_clock_read() functions"))

  - Make arch_timer_read_counter a function, and not just a function pointer

  - Added a command-line option to allow the user to promise that no
    erratum is required (Will)

  - Dropped the internal CPU mask and iterate over the existing
    per-cpu variable (Will)

  - Picked Catalin's Ack, with thanks

[1] https://lore.kernel.org/all/20260508094203.2913880-1-maz@kernel.org/

Marc Zyngier (8):
  clocksource/drivers/arm_arch_timer: Add
    read_sched_clock_is_arch_counter() predicate
  clocksource/drivers/arm_arch_timer: Add a static key indicating the
    need for a runtime workaround
  clocksource/drivers/arm_arch_timer: Convert counter accessors to a
    static key alternative
  clocksource/drivers/arm_arch_timer: Drop the
    arch_counter_get_cnt{p,v}ct_stable() accessors
  clocksource/drivers/arm_arch_timer: Turn arch_timer_read_counter into
    a function
  clocksource/drivers/arm_arch_timer: Add command-line control over the
    counter errata management
  clocksource/drivers/arm_arch_timer: Expose a direct accessor for the
    virtual counter
  arm64: Convert __delay_cycles() to arch_timer_read_vcounter()

 .../admin-guide/kernel-parameters.txt         |  13 ++
 arch/arm64/lib/delay.c                        |   5 +-
 drivers/clocksource/arm_arch_timer.c          | 156 ++++++++++++------
 drivers/perf/arm_pmuv3.c                      |   2 +-
 include/clocksource/arm_arch_timer.h          |  11 +-
 5 files changed, 131 insertions(+), 56 deletions(-)

-- 
2.47.3

[PATCH v3 1/8] clocksource/drivers/arm_arch_timer: Add read_sched_clock_is_arch_counter() predicate

[Marc Zyngier]

The PMUv3 driver really wants to know whether sched_clock() is
backed by the architected timer. However, updates to the timer driver
have broken the check, as there is more than a single accessor
for the counters, depending on the level of brokenness of the HW.

Add a predicate that checks for those accessors, and update the PMUv3
driver to DTRT.

Fixes: 24ee7607b286b ("arm64/arch_timer: Provide noinstr sched_clock_read() functions")
Signed-off-by: Marc Zyngier <maz@kernel.org>
Cc: stable@vger.kernel.org
---
 drivers/clocksource/arm_arch_timer.c | 10 ++++++++++
 drivers/perf/arm_pmuv3.c             |  2 +-
 include/clocksource/arm_arch_timer.h |  8 +++++++-
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 4adf756423de9..c02e97f9b7af5 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -951,6 +951,16 @@ static void __init arch_counter_register(void)
 	sched_clock_register(scr, width, arch_timer_rate);
 }
 
+bool read_sched_clock_is_arch_counter(const struct clock_read_data *crd)
+{
+	u64 (*rd)(void) = crd->read_sched_clock;
+
+	return (rd == raw_counter_get_cntvct_stable	||
+		rd == raw_counter_get_cntpct_stable	||
+		rd == arch_counter_get_cntvct		||
+		rd == arch_counter_get_cntpct);
+}
+
 static void arch_timer_stop(struct clock_event_device *clk)
 {
 	pr_debug("disable IRQ%d cpu #%d\n", clk->irq, smp_processor_id());
diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
index 8014ff766cff5..ed880145d41b3 100644
--- a/drivers/perf/arm_pmuv3.c
+++ b/drivers/perf/arm_pmuv3.c
@@ -1624,7 +1624,7 @@ void arch_perf_update_userpage(struct perf_event *event,
 	do {
 		rd = sched_clock_read_begin(&seq);
 
-		if (rd->read_sched_clock != arch_timer_read_counter)
+		if (!read_sched_clock_is_arch_counter(rd))
 			return;
 
 		userpg->time_mult = rd->mult;
diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h
index 2eda895f19f54..84cdbaffcfb5e 100644
--- a/include/clocksource/arm_arch_timer.h
+++ b/include/clocksource/arm_arch_timer.h
@@ -84,13 +84,15 @@ struct arch_timer_mem {
 	struct arch_timer_mem_frame frame[ARCH_TIMER_MEM_MAX_FRAMES];
 };
 
+struct clock_read_data;
+
 #ifdef CONFIG_ARM_ARCH_TIMER
 
 extern u32 arch_timer_get_rate(void);
 extern u64 (*arch_timer_read_counter)(void);
 extern struct arch_timer_kvm_info *arch_timer_get_kvm_info(void);
 extern bool arch_timer_evtstrm_available(void);
-
+extern bool read_sched_clock_is_arch_counter(const struct clock_read_data *);
 #else
 
 static inline u32 arch_timer_get_rate(void)
@@ -108,6 +110,10 @@ static inline bool arch_timer_evtstrm_available(void)
 	return false;
 }
 
+static bool read_sched_clock_is_arch_counter(const struct clock_read_data *crd)
+{
+	return false;
+}
 #endif
 
 #endif
-- 
2.47.3

[PATCH v3 2/8] clocksource/drivers/arm_arch_timer: Add a static key indicating the need for a runtime workaround

[Marc Zyngier]

In order to decide whether we can read the architected counter without
disabling preemption to look up a workaround, introduce a static key
that denotes whether a workaround is required at all.

The behaviour of this new static key is a bit unusual:

- it starts as 'true', indicating that workarounds are required

- when all possible CPUs have booted at least once, and that it
  has been established that none of them require a workaround,
  the key flips to 'false'

Of course, as long as not all the CPUs have booted once, you
are stuck with slow accessors, but that's what you get for not
sharing your toys.

Things are made a bit complicated because static keys cannot be
flipped from a CPUHP callback. Instead, schedule a deferred work
from there. Yes, this is fun.

Nothing is making use of this stuff yet, but watch this space.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/clocksource/arm_arch_timer.c | 35 ++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index c02e97f9b7af5..8ee317e33c864 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -90,6 +90,8 @@ static int arch_counter_get_width(void)
 /*
  * Architected system timer support.
  */
+static inline bool arch_counter_broken_accessors(void);
+
 static noinstr u64 raw_counter_get_cntpct_stable(void)
 {
 	return __arch_counter_get_cntpct_stable();
@@ -555,10 +557,42 @@ static bool arch_timer_counter_has_wa(void)
 {
 	return atomic_read(&timer_unstable_counter_workaround_in_use);
 }
+
+static DEFINE_STATIC_KEY_TRUE(broken_cnt_accessors);
+
+static inline bool arch_counter_broken_accessors(void)
+{
+	return static_branch_unlikely(&broken_cnt_accessors);
+}
+
+static void enable_direct_accessors(struct work_struct *wk)
+{
+	pr_info("Enabling direct accessors\n");
+	static_branch_disable(&broken_cnt_accessors);
+}
+
+static void arch_timer_set_direct_accessors(void)
+{
+	static DECLARE_WORK(enable_accessors_wk, enable_direct_accessors);
+	int cpu;
+
+	if (!arch_counter_broken_accessors())
+		return;
+
+	/* Each CPU with non-zero IRQ has booted at least once */
+	for_each_cpu(cpu, cpu_possible_mask)
+		if (!per_cpu_ptr(arch_timer_evt, cpu)->irq)
+			return;
+
+	if (!arch_timer_counter_has_wa())
+		schedule_work(&enable_accessors_wk);
+}
 #else
 #define arch_timer_check_ool_workaround(t,a)		do { } while(0)
 #define arch_timer_this_cpu_has_cntvct_wa()		({false;})
 #define arch_timer_counter_has_wa()			({false;})
+static inline bool arch_counter_broken_accessors(void)	{ return false ; }
+#define arch_timer_set_direct_accessors()		do { } while(0)
 #endif /* CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND */
 
 static __always_inline irqreturn_t timer_handler(const int access,
@@ -841,6 +875,7 @@ static int arch_timer_starting_cpu(unsigned int cpu)
 	}
 
 	arch_counter_set_user_access();
+	arch_timer_set_direct_accessors();
 
 	return 0;
 }
-- 
2.47.3

[PATCH v3 3/8] clocksource/drivers/arm_arch_timer: Convert counter accessors to a static key alternative

[Marc Zyngier]

Now that we have a reliable static key to control whether our
counter accessors need to be worked around, use it in these
accessors and simplify the logic that picks which accessor to use.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/clocksource/arm_arch_timer.c | 44 ++++++++++++++--------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 8ee317e33c864..f645aa562895b 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -92,9 +92,12 @@ static int arch_counter_get_width(void)
  */
 static inline bool arch_counter_broken_accessors(void);
 
-static noinstr u64 raw_counter_get_cntpct_stable(void)
+static noinstr u64 raw_counter_get_cntpct(void)
 {
-	return __arch_counter_get_cntpct_stable();
+	if (arch_counter_broken_accessors())
+		return __arch_counter_get_cntpct_stable();
+
+	return __arch_counter_get_cntpct();
 }
 
 static notrace u64 arch_counter_get_cntpct_stable(void)
@@ -108,12 +111,18 @@ static notrace u64 arch_counter_get_cntpct_stable(void)
 
 static noinstr u64 arch_counter_get_cntpct(void)
 {
+	if (arch_counter_broken_accessors())
+		return arch_counter_get_cntpct_stable();
+
 	return __arch_counter_get_cntpct();
 }
 
-static noinstr u64 raw_counter_get_cntvct_stable(void)
+static noinstr u64 raw_counter_get_cntvct(void)
 {
-	return __arch_counter_get_cntvct_stable();
+	if (arch_counter_broken_accessors())
+		return __arch_counter_get_cntvct_stable();
+
+	return __arch_counter_get_cntvct();
 }
 
 static notrace u64 arch_counter_get_cntvct_stable(void)
@@ -127,6 +136,9 @@ static notrace u64 arch_counter_get_cntvct_stable(void)
 
 static noinstr u64 arch_counter_get_cntvct(void)
 {
+	if (arch_counter_broken_accessors())
+		return arch_counter_get_cntvct_stable();
+
 	return __arch_counter_get_cntvct();
 }
 
@@ -950,21 +962,11 @@ static void __init arch_counter_register(void)
 	if ((IS_ENABLED(CONFIG_ARM64) && !is_hyp_mode_available()) ||
 	    arch_timer_uses_ppi == ARCH_TIMER_VIRT_PPI ||
 	    arch_timer_uses_ppi == ARCH_TIMER_HYP_VIRT_PPI) {
-		if (arch_timer_counter_has_wa()) {
-			rd = arch_counter_get_cntvct_stable;
-			scr = raw_counter_get_cntvct_stable;
-		} else {
-			rd = arch_counter_get_cntvct;
-			scr = arch_counter_get_cntvct;
-		}
+		rd = arch_counter_get_cntvct;
+		scr = raw_counter_get_cntvct;
 	} else {
-		if (arch_timer_counter_has_wa()) {
-			rd = arch_counter_get_cntpct_stable;
-			scr = raw_counter_get_cntpct_stable;
-		} else {
-			rd = arch_counter_get_cntpct;
-			scr = arch_counter_get_cntpct;
-		}
+		rd = arch_counter_get_cntpct;
+		scr = raw_counter_get_cntpct;
 	}
 
 	arch_timer_read_counter = rd;
@@ -990,10 +992,8 @@ bool read_sched_clock_is_arch_counter(const struct clock_read_data *crd)
 {
 	u64 (*rd)(void) = crd->read_sched_clock;
 
-	return (rd == raw_counter_get_cntvct_stable	||
-		rd == raw_counter_get_cntpct_stable	||
-		rd == arch_counter_get_cntvct		||
-		rd == arch_counter_get_cntpct);
+	return (rd == raw_counter_get_cntvct	||
+		rd == raw_counter_get_cntpct);
 }
 
 static void arch_timer_stop(struct clock_event_device *clk)
-- 
2.47.3

[PATCH v3 4/8] clocksource/drivers/arm_arch_timer: Drop the arch_counter_get_cnt{p,v}ct_stable() accessors

[Marc Zyngier]

Further simplify the counter accessors by eliminating the *_stable()
ones, which serve little purpose at this stage.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/clocksource/arm_arch_timer.c | 38 +++++++++-------------------
 1 file changed, 12 insertions(+), 26 deletions(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index f645aa562895b..5085e87706f99 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -100,19 +100,12 @@ static noinstr u64 raw_counter_get_cntpct(void)
 	return __arch_counter_get_cntpct();
 }
 
-static notrace u64 arch_counter_get_cntpct_stable(void)
+static notrace u64 arch_counter_get_cntpct(void)
 {
-	u64 val;
-	preempt_disable_notrace();
-	val = __arch_counter_get_cntpct_stable();
-	preempt_enable_notrace();
-	return val;
-}
-
-static noinstr u64 arch_counter_get_cntpct(void)
-{
-	if (arch_counter_broken_accessors())
-		return arch_counter_get_cntpct_stable();
+	if (arch_counter_broken_accessors()) {
+		guard(preempt_notrace)();
+		return __arch_counter_get_cntpct_stable();
+	}
 
 	return __arch_counter_get_cntpct();
 }
@@ -125,19 +118,12 @@ static noinstr u64 raw_counter_get_cntvct(void)
 	return __arch_counter_get_cntvct();
 }
 
-static notrace u64 arch_counter_get_cntvct_stable(void)
+static notrace u64 arch_counter_get_cntvct(void)
 {
-	u64 val;
-	preempt_disable_notrace();
-	val = __arch_counter_get_cntvct_stable();
-	preempt_enable_notrace();
-	return val;
-}
-
-static noinstr u64 arch_counter_get_cntvct(void)
-{
-	if (arch_counter_broken_accessors())
-		return arch_counter_get_cntvct_stable();
+	if (arch_counter_broken_accessors()) {
+		guard(preempt_notrace)();
+		return __arch_counter_get_cntvct_stable();
+	}
 
 	return __arch_counter_get_cntvct();
 }
@@ -342,10 +328,10 @@ void erratum_set_next_event_generic(const int access, unsigned long evt,
 	ctrl &= ~ARCH_TIMER_CTRL_IT_MASK;
 
 	if (access == ARCH_TIMER_PHYS_ACCESS) {
-		cval = evt + arch_counter_get_cntpct_stable();
+		cval = evt + arch_counter_get_cntpct();
 		write_sysreg(cval, cntp_cval_el0);
 	} else {
-		cval = evt + arch_counter_get_cntvct_stable();
+		cval = evt + arch_counter_get_cntvct();
 		write_sysreg(cval, cntv_cval_el0);
 	}
 
-- 
2.47.3

[PATCH v3 5/8] clocksource/drivers/arm_arch_timer: Turn arch_timer_read_counter into a function

[Marc Zyngier]

Now that we control the workarounds applied to the counters with
a static key, we can also do the same thing for the selection
between virtual and physical counters, as this selection is
done on the boot CPU, where we have the freedom to flip static
keys.

With this, there is no need for arch_timer_read_counter to be
a function pointer, and we can directly promote it to an actual
function.

While we're at it, apply the same static key treatment to the raw_*
accessors.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/clocksource/arm_arch_timer.c | 38 +++++++++++++++++-----------
 include/clocksource/arm_arch_timer.h |  2 +-
 2 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 5085e87706f99..747f51d9225c2 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -134,9 +134,21 @@ static notrace u64 arch_counter_get_cntvct(void)
  * to exist on arm64. arm doesn't use this before DT is probed so even
  * if we don't have the cp15 accessors we won't have a problem.
  */
-u64 (*arch_timer_read_counter)(void) __ro_after_init = arch_counter_get_cntvct;
+static DEFINE_STATIC_KEY_TRUE(arch_counter_is_virtual);
+
+u64 notrace arch_timer_read_counter(void)
+{
+	return static_branch_likely(&arch_counter_is_virtual) ?
+		arch_counter_get_cntvct() : arch_counter_get_cntpct();
+}
 EXPORT_SYMBOL_GPL(arch_timer_read_counter);
 
+static u64 noinstr raw_arch_timer_read_counter(void)
+{
+	return static_branch_likely(&arch_counter_is_virtual) ?
+		raw_counter_get_cntvct() : raw_counter_get_cntpct();
+}
+
 static u64 arch_counter_read(struct clocksource *cs)
 {
 	return arch_timer_read_counter();
@@ -940,22 +952,19 @@ struct arch_timer_kvm_info *arch_timer_get_kvm_info(void)
 
 static void __init arch_counter_register(void)
 {
-	u64 (*scr)(void);
-	u64 (*rd)(void);
 	u64 start_count;
 	int width;
 
-	if ((IS_ENABLED(CONFIG_ARM64) && !is_hyp_mode_available()) ||
-	    arch_timer_uses_ppi == ARCH_TIMER_VIRT_PPI ||
-	    arch_timer_uses_ppi == ARCH_TIMER_HYP_VIRT_PPI) {
-		rd = arch_counter_get_cntvct;
-		scr = raw_counter_get_cntvct;
-	} else {
-		rd = arch_counter_get_cntpct;
-		scr = raw_counter_get_cntpct;
+	switch (arch_timer_uses_ppi) {
+	case ARCH_TIMER_PHYS_SECURE_PPI:
+	case ARCH_TIMER_PHYS_NONSECURE_PPI:
+	case ARCH_TIMER_HYP_PPI:
+		static_branch_disable(&arch_counter_is_virtual);
+		break;
+	default:
+		break;
 	}
 
-	arch_timer_read_counter = rd;
 	clocksource_counter.vdso_clock_mode = vdso_default;
 
 	width = arch_counter_get_width();
@@ -971,15 +980,14 @@ static void __init arch_counter_register(void)
 	timecounter_init(&arch_timer_kvm_info.timecounter,
 			 &cyclecounter, start_count);
 
-	sched_clock_register(scr, width, arch_timer_rate);
+	sched_clock_register(raw_arch_timer_read_counter, width, arch_timer_rate);
 }
 
 bool read_sched_clock_is_arch_counter(const struct clock_read_data *crd)
 {
 	u64 (*rd)(void) = crd->read_sched_clock;
 
-	return (rd == raw_counter_get_cntvct	||
-		rd == raw_counter_get_cntpct);
+	return (rd == raw_arch_timer_read_counter);
 }
 
 static void arch_timer_stop(struct clock_event_device *clk)
diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h
index 84cdbaffcfb5e..8deb815ed0a08 100644
--- a/include/clocksource/arm_arch_timer.h
+++ b/include/clocksource/arm_arch_timer.h
@@ -89,7 +89,7 @@ struct clock_read_data;
 #ifdef CONFIG_ARM_ARCH_TIMER
 
 extern u32 arch_timer_get_rate(void);
-extern u64 (*arch_timer_read_counter)(void);
+extern u64 arch_timer_read_counter(void);
 extern struct arch_timer_kvm_info *arch_timer_get_kvm_info(void);
 extern bool arch_timer_evtstrm_available(void);
 extern bool read_sched_clock_is_arch_counter(const struct clock_read_data *);
-- 
2.47.3

[PATCH v3 6/8] clocksource/drivers/arm_arch_timer: Add command-line control over the counter errata management

[Marc Zyngier]

While we now have a safe way to use "fast" accessors once all CPUs
have booted, this leaves people booting with maxcpus= on non-broken
hardware stuck with the "slow" counter accessors.

Give these people a way out by adding a new command-line parameter
aptly named clocksource.arm_arch_timer.cnt_errata, which allows
the user to promise that no erratum handling is required for the
counters by setting this value to 0.

Warnings will be emitted if the user has over-promised.

Suggested-by; Will Deacon <will@kernel.org>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 Documentation/admin-guide/kernel-parameters.txt | 13 +++++++++++++
 drivers/clocksource/arm_arch_timer.c            | 17 ++++++++++++++++-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index b5493a7f8f228..f0e6534269c2b 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -830,6 +830,19 @@ Kernel parameters
 			loops can be debugged more effectively on production
 			systems.
 
+	clocksource.arm_arch_timer.cnt_errata=
+			[ARM64,EARLY]
+			Format: <bool>
+			Enable/disable the counter errata management.
+			Enabling it switches over to fast accessors once it is
+			known that no CPU requires any workaround while reading
+			the counters.
+			Disabling it will bypass workarounds when reading the
+			counters, even if not all the CPUs have been probed.
+			Warnings will be produced if the need for a workaround
+			is detected.
+			Default is enabled.
+
 	clocksource.verify_n_cpus= [KNL]
 			Limit the number of CPUs checked for clocksources
 			marked with CLOCK_SOURCE_VERIFY_PERCPU that
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 747f51d9225c2..b3b31d4f4815f 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -500,8 +500,10 @@ void arch_timer_enable_workaround(const struct arch_timer_erratum_workaround *wa
 			per_cpu(timer_unstable_counter_workaround, i) = wa;
 	}
 
-	if (wa->read_cntvct_el0 || wa->read_cntpct_el0)
+	if (wa->read_cntvct_el0 || wa->read_cntpct_el0) {
+		WARN_ON_ONCE(!arch_counter_broken_accessors());
 		atomic_set(&timer_unstable_counter_workaround_in_use, 1);
+	}
 
 	/*
 	 * Don't use the vdso fastpath if errata require using the
@@ -597,12 +599,22 @@ static void arch_timer_set_direct_accessors(void)
 	if (!arch_timer_counter_has_wa())
 		schedule_work(&enable_accessors_wk);
 }
+
+static bool cnt_errata_config __initdata = true;
+
+static int __init early_cnt_errata(char *buf)
+{
+	return kstrtobool(buf, &cnt_errata_config);
+}
+early_param("clocksource.arm_arch_timer.cnt_errata", early_cnt_errata);
 #else
 #define arch_timer_check_ool_workaround(t,a)		do { } while(0)
 #define arch_timer_this_cpu_has_cntvct_wa()		({false;})
 #define arch_timer_counter_has_wa()			({false;})
 static inline bool arch_counter_broken_accessors(void)	{ return false ; }
 #define arch_timer_set_direct_accessors()		do { } while(0)
+#define enable_direct_accessors(w)			do { } while(0)
+#define cnt_errata_config				false
 #endif /* CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND */
 
 static __always_inline irqreturn_t timer_handler(const int access,
@@ -955,6 +967,9 @@ static void __init arch_counter_register(void)
 	u64 start_count;
 	int width;
 
+	if (!cnt_errata_config)
+		enable_direct_accessors(NULL);
+
 	switch (arch_timer_uses_ppi) {
 	case ARCH_TIMER_PHYS_SECURE_PPI:
 	case ARCH_TIMER_PHYS_NONSECURE_PPI:
-- 
2.47.3

[PATCH v3 7/8] clocksource/drivers/arm_arch_timer: Expose a direct accessor for the virtual counter

[Marc Zyngier]

We allow access to the architected counter via arch_timer_read_counter().
However, this accessor can either be the virtual or the physical
view of the counter, depending on how the kernel has been booted.

At the same time, we have some architectural features (such as WFIT,
WFET) that rely on the virtual counter, and nothing else.

If implementations were perfect, we'd rely on reading CNTVCT_EL0,
and be done with it. However, we have a bunch of broken implementations
in the wild, which rely on preemption being disabled and other
costly workarounds.

In order to provide decent performance on non-broken HW while still
supporting the legacy horrors, expose arch_timer_read_vcounter() as
a new helper that hides this complexity. Obviously, this is simply
a global alias of arch_counter_get_cntvct().

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/clocksource/arm_arch_timer.c | 2 ++
 include/clocksource/arm_arch_timer.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index b3b31d4f4815f..9c7501ce57073 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -128,6 +128,8 @@ static notrace u64 arch_counter_get_cntvct(void)
 	return __arch_counter_get_cntvct();
 }
 
+u64 arch_timer_read_vcounter(void) __attribute__((alias("arch_counter_get_cntvct")));
+
 /*
  * Default to cp15 based access because arm64 uses this function for
  * sched_clock() before DT is probed and the cp15 method is guaranteed
diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h
index 8deb815ed0a08..32fd531ac009f 100644
--- a/include/clocksource/arm_arch_timer.h
+++ b/include/clocksource/arm_arch_timer.h
@@ -90,6 +90,7 @@ struct clock_read_data;
 
 extern u32 arch_timer_get_rate(void);
 extern u64 arch_timer_read_counter(void);
+extern u64 arch_timer_read_vcounter(void);
 extern struct arch_timer_kvm_info *arch_timer_get_kvm_info(void);
 extern bool arch_timer_evtstrm_available(void);
 extern bool read_sched_clock_is_arch_counter(const struct clock_read_data *);
-- 
2.47.3

[PATCH v3 8/8] arm64: Convert __delay_cycles() to arch_timer_read_vcounter()

[Marc Zyngier]

Relax the need for disabling preemption in __delay_cycles() by
using arch_timer_read_vcounter(), which will disable preemption
only when this is actually required.

Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/lib/delay.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/lib/delay.c b/arch/arm64/lib/delay.c
index e278e060e78a9..a667df920697d 100644
--- a/arch/arm64/lib/delay.c
+++ b/arch/arm64/lib/delay.c
@@ -32,10 +32,9 @@ static inline unsigned long xloops_to_cycles(unsigned long xloops)
  * Note that userspace cannot change the offset behind our back either,
  * as the vcpu mutex is held as long as KVM_RUN is in progress.
  */
-static cycles_t notrace __delay_cycles(void)
+static cycles_t __delay_cycles(void)
 {
-	guard(preempt_notrace)();
-	return __arch_counter_get_cntvct_stable();
+	return arch_timer_read_vcounter();
 }
 
 void __delay(unsigned long cycles)
-- 
2.47.3