Re: [Arm] Re: [PATCH v1 11/15] xen/riscv: introduce ns_to_ticks()

[Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>]

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