On 24.12.2025 18:03, Oleksii Kurochko wrote:
> Signed-off-by: Oleksii Kurochko <[email protected]>
> ---
> xen/arch/riscv/include/asm/time.h | 5 +++++
> 1 file changed, 5 insertions(+)
Looks okay and read to go in as is (no dependencies on earlier patches afaics),
but:
> --- a/xen/arch/riscv/include/asm/time.h
> +++ b/xen/arch/riscv/include/asm/time.h
> @@ -29,6 +29,11 @@ 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));
> +}
It's hard to see what's arch-dependent about this or ticks_to_ns(). They're
similar but not identical to Arm's version, and I actually wonder why that
difference exists. Questions to Arm people:
1) Why are they out-of-line functions there?
2) Why the involvement of the constant 1000 there? 1000 * cpu_khz can
actually overflow in 32 bits. The forms above aren't prone to such an
issue.
If the delta isn't justified, I think we'd better put RISC-V's functions in
common code (xen/time.h). They're not presently needed by x86, but as
inline functions they also shouldn't do any harm.
Jan