On Wed, 21 Jan 2026, Volodymyr Babchuk wrote: > > 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? > > 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.
