Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
 xen/arch/riscv/include/asm/time.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/xen/arch/riscv/include/asm/time.h b/xen/arch/riscv/include/asm/time.h
index 63bdd471ccac..3d013a3ace0f 100644
--- 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));
+}
+
 void preinit_xen_time(void);
 
 #endif /* ASM__RISCV__TIME_H */
-- 
2.52.0

On 24.12.2025 18:03, Oleksii Kurochko wrote:
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
>  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

Hi Jan,

Jan Beulich <jbeulich@suse.com> writes:

> On 24.12.2025 18:03, Oleksii Kurochko wrote:
>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>> ---
>>  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?

That's interesting question. According to git blame this is how it was
introduced in 2012 and after that no one touched this part. Original
patch had cntfrq defined as `static`, this explains why these functions
were declared out-of-line.

> 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.

Patch "xen: move XEN_SYSCTL_physinfo, XEN_SYSCTL_numainfo and
XEN_SYSCTL_topologyinfo to common code" (096578b4e48) changed hz to
khz. This added that 1000 multiplication. Also this patch removed
`static` qualifier from the counter variable.

Anyways, latest ARM ARM suggests that timer frequency should be fixed at
1GHz, which is shy of 32-bit overflow. So most new platforms will be
fine. And older platforms had much lower frequencies.

> 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.

I'm mere reviewer, but I agree that proposed approach is better and more
resilient.

-- 
WBR, Volodymyr

On Wed, 21 Jan 2026, Volodymyr Babchuk wrote:
> > On 24.12.2025 18:03, Oleksii Kurochko wrote:
> >> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> >> ---
> >>  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?
> 
> That's interesting question. According to git blame this is how it was
> introduced in 2012 and after that no one touched this part. Original
> patch had cntfrq defined as `static`, this explains why these functions
> were declared out-of-line.
> 
> > 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.
> 
> Patch "xen: move XEN_SYSCTL_physinfo, XEN_SYSCTL_numainfo and
> XEN_SYSCTL_topologyinfo to common code" (096578b4e48) changed hz to
> khz. This added that 1000 multiplication. Also this patch removed
> `static` qualifier from the counter variable.
> 
> Anyways, latest ARM ARM suggests that timer frequency should be fixed at
> 1GHz, which is shy of 32-bit overflow. So most new platforms will be
> fine. And older platforms had much lower frequencies.
> 
> > 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.
> 
> I'm mere reviewer, but I agree that proposed approach is better and more
> resilient.

Yes I agree too.