Re: [PATCH v6 05/14] xen/time: move ticks<->ns helpers to common code

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Oleksii

> On 26 Feb 2026, at 12:51, Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx> wrote:
> 
> ticks_to_ns() and ns_to_ticks() are not architecture-specific, so provide a
> common implementation that is more resilient to overflow than the historical
> Arm version. This is not a practical issue for Arm, as the latest ARM ARM
> that timer frequency should be fixed at 1 GHz and older platforms used much
> lower rates, which is shy of 32-bit overflow. As the helpers are declared
> as static inline, they should not affect x86, which does not use them.
> 
> On Arm, these helpers were historically implemented as out-of-line functions
> because the counter frequency was originally defined as static and unavailable
> to headers [1]. Later changes [2] removed this restriction, but the helpers
> remained unchanged. Now they can be implemented as static inline without any
> issues.
> 
> Centralising the helpers avoids duplication and removes subtle differences
> between architectures while keeping the implementation simple.
> 
> Drop redundant <asm/time.h> includes where <xen/time.h> already pulls it in.
> 
> No functional change is intended.
> 
> [1] ddee56dc2994 arm: driver for the generic timer for ARMv7
> [2] 096578b4e489 xen: move XEN_SYSCTL_physinfo, XEN_SYSCTL_numainfo and
>                      XEN_SYSCTL_topologyinfo to common code
> 
> Suggested-by: Jan Beulich <jbeulich@xxxxxxxx>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Computation is changing a bit from MS to S removing the 1000 factor on arm
but i do not think this would have an impact so:

Acked-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>

Cheers
Bertrand

> ---
> Changes in v4-v6:
> - Nothing changed. Only rebase.
> ---
> Changes in v3:
> - Add Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>.
> ---
> Changes in v2:
> - Move ns_to_ticks() and ticks_to_ns() to common code.
> ---
> xen/arch/arm/include/asm/time.h   |  3 ---
> xen/arch/arm/time.c               | 11 -----------
> xen/arch/arm/vtimer.c             |  2 +-
> xen/arch/riscv/include/asm/time.h |  5 -----
> xen/arch/riscv/time.c             |  1 +
> xen/include/xen/time.h            | 11 +++++++++++
> 6 files changed, 13 insertions(+), 20 deletions(-)
> 
> diff --git a/xen/arch/arm/include/asm/time.h b/xen/arch/arm/include/asm/time.h
> index 49ad8c1a6d47..c194dbb9f52d 100644
> --- a/xen/arch/arm/include/asm/time.h
> +++ b/xen/arch/arm/include/asm/time.h
> @@ -101,9 +101,6 @@ extern void init_timer_interrupt(void);
> /* Counter value at boot time */
> extern uint64_t boot_count;
> 
> -extern s_time_t ticks_to_ns(uint64_t ticks);
> -extern uint64_t ns_to_ticks(s_time_t ns);
> -
> void preinit_xen_time(void);
> 
> void force_update_vcpu_system_time(struct vcpu *v);
> diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
> index cc3fcf47b66a..a12912a106a0 100644
> --- a/xen/arch/arm/time.c
> +++ b/xen/arch/arm/time.c
> @@ -27,7 +27,6 @@
> #include <asm/cpufeature.h>
> #include <asm/platform.h>
> #include <asm/system.h>
> -#include <asm/time.h>
> #include <asm/vgic.h>
> 
> uint64_t __read_mostly boot_count;
> @@ -47,16 +46,6 @@ unsigned int timer_get_irq(enum timer_ppi ppi)
>     return timer_irq[ppi];
> }
> 
> -/*static inline*/ s_time_t ticks_to_ns(uint64_t ticks)
> -{
> -    return muldiv64(ticks, SECONDS(1), 1000 * cpu_khz);
> -}
> -
> -/*static inline*/ uint64_t ns_to_ticks(s_time_t ns)
> -{
> -    return muldiv64(ns, 1000 * cpu_khz, SECONDS(1));
> -}
> -
> static __initdata struct dt_device_node *timer;
> 
> #ifdef CONFIG_ACPI
> diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
> index d2124b175521..2e85ff2b6e62 100644
> --- a/xen/arch/arm/vtimer.c
> +++ b/xen/arch/arm/vtimer.c
> @@ -12,13 +12,13 @@
> #include <xen/lib.h>
> #include <xen/perfc.h>
> #include <xen/sched.h>
> +#include <xen/time.h>
> #include <xen/timer.h>
> 
> #include <asm/cpregs.h>
> #include <asm/div64.h>
> #include <asm/irq.h>
> #include <asm/regs.h>
> -#include <asm/time.h>
> #include <asm/vgic.h>
> #include <asm/vreg.h>
> #include <asm/vtimer.h>
> diff --git a/xen/arch/riscv/include/asm/time.h 
> b/xen/arch/riscv/include/asm/time.h
> index 1e7801e2ea0e..be3875b9984e 100644
> --- a/xen/arch/riscv/include/asm/time.h
> +++ b/xen/arch/riscv/include/asm/time.h
> @@ -24,11 +24,6 @@ static inline cycles_t get_cycles(void)
>     return csr_read(CSR_TIME);
> }
> 
> -static inline s_time_t ticks_to_ns(uint64_t ticks)
> -{
> -    return muldiv64(ticks, MILLISECS(1), cpu_khz);
> -}
> -
> void preinit_xen_time(void);
> 
> #endif /* ASM__RISCV__TIME_H */
> diff --git a/xen/arch/riscv/time.c b/xen/arch/riscv/time.c
> index e962f8518d78..2c7af0a5d63b 100644
> --- a/xen/arch/riscv/time.c
> +++ b/xen/arch/riscv/time.c
> @@ -4,6 +4,7 @@
> #include <xen/init.h>
> #include <xen/lib.h>
> #include <xen/sections.h>
> +#include <xen/time.h>
> #include <xen/types.h>
> 
> unsigned long __ro_after_init cpu_khz; /* CPU clock frequency in kHz. */
> diff --git a/xen/include/xen/time.h b/xen/include/xen/time.h
> index fe0d7a578a99..2185dd26a439 100644
> --- a/xen/include/xen/time.h
> +++ b/xen/include/xen/time.h
> @@ -8,6 +8,7 @@
> #ifndef __XEN_TIME_H__
> #define __XEN_TIME_H__
> 
> +#include <xen/muldiv64.h>
> #include <xen/types.h>
> #include <public/xen.h>
> 
> @@ -75,6 +76,16 @@ extern void send_timer_event(struct vcpu *v);
> 
> void domain_set_time_offset(struct domain *d, int64_t time_offset_seconds);
> 
> +static inline s_time_t ticks_to_ns(uint64_t ticks)
> +{
> +    return muldiv64(ticks, MILLISECS(1), cpu_khz);
> +}
> +
> +static inline uint64_t ns_to_ticks(s_time_t ns)
> +{
> +    return muldiv64(ns, cpu_khz, MILLISECS(1));
> +}
> +
> #include <asm/time.h>
> 
> #endif /* __XEN_TIME_H__ */
> -- 
> 2.53.0
>