Re: [PATCH v1 01/14] xen/riscv: implement get_s_time()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 08.04.2025 17:57, Oleksii Kurochko wrote:
> @@ -23,6 +24,11 @@ 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, SECONDS(1), 1000 * cpu_khz);
> +}

Why the extra multiplication by 1000? I.e. why not
"muldiv64(ticks, MILLISECONDS(1), cpu_khz)", getting away with just one
multiplication and a reduced risk of encountering intermediate overflow
(affecting only hypothetical above 4THz CPUs then)?

> --- a/xen/arch/riscv/time.c
> +++ b/xen/arch/riscv/time.c
> @@ -4,10 +4,17 @@
>  #include <xen/init.h>
>  #include <xen/lib.h>
>  #include <xen/sections.h>
> +#include <xen/types.h>
>  
>  unsigned long __ro_after_init cpu_khz; /* CPU clock frequency in kHz. */
>  uint64_t __ro_after_init boot_clock_cycles;
>  
> +s_time_t get_s_time(void)
> +{
> +    uint64_t ticks = get_cycles() - boot_clock_cycles;
> +    return ticks_to_ns(ticks);

Nit: Blank line between declaration(s) and statement(s) please, as well as
ahead of the main "return" of a function.

Happy to make both adjustments upon committing, so long as you agree; then:
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan