RE: [patch 19/48] clockevents: Provide support for clocksource coupled comparators

[Michael Kelley]

From: Thomas Gleixner <tglx@xxxxxxxxxx> Sent: Tuesday, February 24, 2026 8:37 AM
> 
> Some clockevent devices are coupled to the system clocksource by
> implementing a less than or equal comparator which compares the programmed
> absolute expiry time against the underlying time counter.

I've been playing with this in linux-next, and particularly to set up the Hyper-V
TSC page clocksource and Hyper-V timer as coupled. Most Hyper-V guests these days
are running on hardware that allows using the TSC directly as the clocksource. But
even if the Hyper-V TSC page clocksource isn't used, the timer is still the Hyper-V
timer, so the coupling isn't active. However, SEV-SNP CoCo VMs on Hyper-V must
use both the Hyper-V TSC page clocksource and the Hyper-V timer, so they would
benefit from coupling. It's a nice idea!

In doing the Hyper-V clocksource and timer coupling, I encountered two issues as
noted below.

> 
> The timekeeping core provides a function to convert and absolute
> CLOCK_MONOTONIC based expiry time to a absolute clock cycles time which can
> be directly fed into the comparator. That spares two time reads in the next
> event progamming path, one to convert the absolute nanoseconds time to a
> delta value and the other to convert the delta value back to a absolute
> time value suitable for the comparator.
> 
> Provide a new clocksource callback which takes the absolute cycle value and
> wire it up in clockevents_program_event(). Similar to clocksources allow
> architectures to inline the rearm operation.
> 
> Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxx>
> ---
>  include/linux/clockchips.h |    7 +++++--
>  kernel/time/Kconfig        |    4 ++++
>  kernel/time/clockevents.c  |   44 +++++++++++++++++++++++++++++++++++++++---
> --
>  3 files changed, 48 insertions(+), 7 deletions(-)
> 
> --- a/include/linux/clockchips.h
> +++ b/include/linux/clockchips.h
> @@ -43,8 +43,9 @@ enum clock_event_state {
>  /*
>   * Clock event features
>   */
> -# define CLOCK_EVT_FEAT_PERIODIC	0x000001
> -# define CLOCK_EVT_FEAT_ONESHOT		0x000002
> +# define CLOCK_EVT_FEAT_PERIODIC		0x000001
> +# define CLOCK_EVT_FEAT_ONESHOT			0x000002
> +# define CLOCK_EVT_FEAT_CLOCKSOURCE_COUPLED	0x000004
> 
>  /*
>   * x86(64) specific (mis)features:
> @@ -100,6 +101,7 @@ struct clock_event_device {
>  	void			(*event_handler)(struct clock_event_device *);
>  	int			(*set_next_event)(unsigned long evt, struct clock_event_device *);
>  	int			(*set_next_ktime)(ktime_t expires, struct clock_event_device *);
> +	void			(*set_next_coupled)(u64 cycles, struct clock_event_device *);
>  	ktime_t			next_event;
>  	u64			max_delta_ns;
>  	u64			min_delta_ns;
> @@ -107,6 +109,7 @@ struct clock_event_device {
>  	u32			shift;
>  	enum clock_event_state	state_use_accessors;
>  	unsigned int		features;
> +	enum clocksource_ids	cs_id;
>  	unsigned long		retries;
> 
>  	int			(*set_state_periodic)(struct clock_event_device *);
> --- a/kernel/time/Kconfig
> +++ b/kernel/time/Kconfig
> @@ -50,6 +50,10 @@ config GENERIC_CLOCKEVENTS_MIN_ADJUST
>  config GENERIC_CLOCKEVENTS_COUPLED
>  	bool
> 
> +config GENERIC_CLOCKEVENTS_COUPLED_INLINE
> +	select GENERIC_CLOCKEVENTS_COUPLED
> +	bool
> +
>  # Generic update of CMOS clock
>  config GENERIC_CMOS_UPDATE
>  	bool
> --- a/kernel/time/clockevents.c
> +++ b/kernel/time/clockevents.c
> @@ -292,6 +292,38 @@ static int clockevents_program_min_delta
> 
>  #endif /* CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST */
> 
> +#ifdef CONFIG_GENERIC_CLOCKEVENTS_COUPLED
> +#ifdef CONFIG_GENERIC_CLOCKEVENTS_COUPLED_INLINE
> +#include <asm/clock_inlined.h>
> +#else
> +static __always_inline void
> +arch_inlined_clockevent_set_next_coupled(u64 u64 cycles, struct clock_event_device *dev) { }

Typo -- there are two "u64" in a row, so it doesn't compile if COUPLED is selected
but COUPLED_INLINE is not.

> +#endif
> +
> +static inline bool clockevent_set_next_coupled(struct clock_event_device *dev, ktime_t expires)
> +{
> +	u64 cycles;
> +
> +	if (unlikely(!(dev->features & CLOCK_EVT_FEAT_CLOCKSOURCE_COUPLED)))
> +		return false;
> +
> +	if (unlikely(!ktime_expiry_to_cycles(dev->cs_id, expires, &cycles)))
> +		return false;
> +
> +	if (IS_ENABLED(CONFIG_GENERIC_CLOCKEVENTS_COUPLED_INLINE))

Since COUPLED_INLINE is always selected for x64, there's no way to add the Hyper-V
clockevent that is coupled but not inline. Adding the machinery to allow a second
inline clockevent type may not be worth it, but adding a second coupled but not
inline clockevent type on x64 should be supported. Thoughts?

After fixing the u64 typo, and temporarily not always selecting COUPLED_INLINE in
arch/x86/Kconfig, the coupled Hyper-V TSC page clocksource and timer seem to work
correctly, though I'm still doing some testing. I'm also working on counting the number
of time reads to confirm the expected benefit.

Michael

> +		arch_inlined_clockevent_set_next_coupled(cycles, dev);
> +	else
> +		dev->set_next_coupled(cycles, dev);
> +	return true;
> +}
> +
> +#else
> +static inline bool clockevent_set_next_coupled(struct clock_event_device *dev, ktime_t expires)
> +{
> +	return false;
> +}
> +#endif
> +
>  /**
>   * clockevents_program_event - Reprogram the clock event device.
>   * @dev:	device to program
> @@ -300,11 +332,10 @@ static int clockevents_program_min_delta
>   *
>   * Returns 0 on success, -ETIME when the event is in the past.
>   */
> -int clockevents_program_event(struct clock_event_device *dev, ktime_t expires,
> -			      bool force)
> +int clockevents_program_event(struct clock_event_device *dev, ktime_t expires, bool force)
>  {
> -	unsigned long long clc;
>  	int64_t delta;
> +	u64 cycles;
>  	int rc;
> 
>  	if (WARN_ON_ONCE(expires < 0))
> @@ -323,6 +354,9 @@ int clockevents_program_event(struct clo
>  	if (unlikely(dev->features & CLOCK_EVT_FEAT_HRTIMER))
>  		return dev->set_next_ktime(expires, dev);
> 
> +	if (likely(clockevent_set_next_coupled(dev, expires)))
> +		return 0;
> +
>  	delta = ktime_to_ns(ktime_sub(expires, ktime_get()));
>  	if (delta <= 0)
>  		return force ? clockevents_program_min_delta(dev) : -ETIME;
> @@ -330,8 +364,8 @@ int clockevents_program_event(struct clo
>  	delta = min(delta, (int64_t) dev->max_delta_ns);
>  	delta = max(delta, (int64_t) dev->min_delta_ns);
> 
> -	clc = ((unsigned long long) delta * dev->mult) >> dev->shift;
> -	rc = dev->set_next_event((unsigned long) clc, dev);
> +	cycles = ((u64)delta * dev->mult) >> dev->shift;
> +	rc = dev->set_next_event((unsigned long) cycles, dev);
> 
>  	return (rc && force) ? clockevents_program_min_delta(dev) : rc;
>  }
>