Re: [PATCH 1/2] qemu/timer: introduce time dilation factor

[Paolo Bonzini]

Il mer 11 giu 2025, 20:55 Pierrick Bouvier  ha
scritto:

> > 1) different clocks have different starts, so the clock_start must be
> > stored per clock type
> >
>
> I was not sure if several clocks are used, so I'll simply use static in
> concerned functions.
>

Ok.

The cycles counter is also problematic, so perhaps make dilation work only
with TCG?

> 2) dilation must be applied to timers too.
>
>  From what I saw, timers either use clock function (already dilated), or
> rely on cpu_get_host_ticks().
> Do you suggest to dilate time returned by cpu_get_host_ticks(), or
> something different?
>

Right. It just seemed that something was missing... and indeed that's
deadline computation: it must convert the dilated deadline to host
nanoseconds, i.e. do a division instead of a multiply.

Paolo

Re: [PATCH 1/2] qemu/timer: introduce time dilation factor

[Pierrick Bouvier]

On 6/11/25 12:22 AM, Paolo Bonzini wrote:



Il mar 10 giu 2025, 23:28 Pierrick Bouvier > ha scritto:


This factor is applied to time spent since we read clock for the first
time. It impacts value returned by get_clock() and get_clock_realtime().


Sounds like a good idea, however it needs a couple changes:

1) different clocks have different starts, so the clock_start must be 
stored per clock type




I was not sure if several clocks are used, so I'll simply use static in 
concerned functions.



2) dilation must be applied to timers too.



From what I saw, timers either use clock function (already dilated), or 
rely on cpu_get_host_ticks().
Do you suggest to dilate time returned by cpu_get_host_ticks(), or 
something different?


As to the option, it's not immediately clear is <1 is a speed up or a 
slow down. Maybe speed-factor=N is more clearly speeding up things if N>1?


+    g_assert(now >= clock_start);



You're right, it's not obvious.
I'm ok with speed-factor name.



The assertion is not needed, and can even fail in cases involving 
daylight savings time; perhaps you can assert that the result is 
positive instead?


+    if (!clock_time_dilation) {
+        return now;
+    }


Just initialize it to 1?



For exactly the same reason you mention under, possible loss of 
precision. If my math is correct, we can only have a precision of 256 
nanoseconds at this epoch time.
Adding an intermediate cast solves this though, so we can have a default 
value of 1.



+    return clock_start + (now - clock_start) * clock_time_dilation;


Please cast back to integer after multiplying. Adding back clock_start 
in floating point format loses precision (doubles have only 53 bits of 
precision; seconds use 32 of them if the base is 1970, and nanoseconds 
don't have the 30 bits they need).




I'll add the cast.


+}
+
  /*
   * Low level clock functions
   */
@@ -811,11 +823,9 @@ static inline int64_t get_clock_realtime(void)
      struct timeval tv;

      gettimeofday(&tv, NULL);
-    return tv.tv_sec * 10LL + (tv.tv_usec * 1000);
+    return dilate_time(tv.tv_sec * 10LL + (tv.tv_usec * 1000));
  }

-extern int64_t clock_start;
-
  /* Warning: don't insert tracepoints into these functions, they are
     also used by simpletrace backend and tracepoints would cause
     an infinite recursion! */
@@ -826,7 +836,7 @@ static inline int64_t get_clock(void)
  {
      LARGE_INTEGER ti;
      QueryPerformanceCounter(&ti);
-    return muldiv64(ti.QuadPart, NANOSECONDS_PER_SECOND, clock_freq);
+    dilate_time(muldiv64(ti.QuadPart, NANOSECONDS_PER_SECOND,
clock_freq));


Missing "return".



Thanks, I caught this quickly after sending also.


Paolo

  }

  #else
@@ -838,10 +848,10 @@ static inline int64_t get_clock(void)
      if (use_rt_clock) {
          struct timespec ts;
          clock_gettime(CLOCK_MONOTONIC, &ts);
-        return ts.tv_sec * 10LL + ts.tv_nsec;
+        return dilate_time(ts.tv_sec * 10LL + ts.tv_nsec);
      } else {
          /* XXX: using gettimeofday leads to problems if the date
-           changes, so it should be avoided. */
+           changes, so it should be avoided. Time is already
dilated. */
          return get_clock_realtime();
      }
  }
diff --git a/util/qemu-timer-common.c b/util/qemu-timer-common.c
index cc1326f7264..d8895aaccad 100644
--- a/util/qemu-timer-common.c
+++ b/util/qemu-timer-common.c
@@ -28,6 +28,7 @@
  /* real time host monotonic timer */

  int64_t clock_start;
+double clock_time_dilation;

  #ifdef _WIN32

-- 
2.47.2




Thanks,
Pierrick

Re: [PATCH 1/2] qemu/timer: introduce time dilation factor

[Paolo Bonzini]

Il mar 10 giu 2025, 23:28 Pierrick Bouvier  ha
scritto:

> This factor is applied to time spent since we read clock for the first
> time. It impacts value returned by get_clock() and get_clock_realtime().
>

Sounds like a good idea, however it needs a couple changes:

1) different clocks have different starts, so the clock_start must be
stored per clock type

2) dilation must be applied to timers too.

As to the option, it's not immediately clear is <1 is a speed up or a slow
down. Maybe speed-factor=N is more clearly speeding up things if N>1?

+g_assert(now >= clock_start);
>

The assertion is not needed, and can even fail in cases involving daylight
savings time; perhaps you can assert that the result is positive instead?

+if (!clock_time_dilation) {
> +return now;
> +}
>

Just initialize it to 1?

+return clock_start + (now - clock_start) * clock_time_dilation;
>

Please cast back to integer after multiplying. Adding back clock_start in
floating point format loses precision (doubles have only 53 bits of
precision; seconds use 32 of them if the base is 1970, and nanoseconds
don't have the 30 bits they need).

+}
> +
>  /*
>   * Low level clock functions
>   */
> @@ -811,11 +823,9 @@ static inline int64_t get_clock_realtime(void)
>  struct timeval tv;
>
>  gettimeofday(&tv, NULL);
> -return tv.tv_sec * 10LL + (tv.tv_usec * 1000);
> +return dilate_time(tv.tv_sec * 10LL + (tv.tv_usec * 1000));
>  }
>
> -extern int64_t clock_start;
> -
>  /* Warning: don't insert tracepoints into these functions, they are
> also used by simpletrace backend and tracepoints would cause
> an infinite recursion! */
> @@ -826,7 +836,7 @@ static inline int64_t get_clock(void)
>  {
>  LARGE_INTEGER ti;
>  QueryPerformanceCounter(&ti);
> -return muldiv64(ti.QuadPart, NANOSECONDS_PER_SECOND, clock_freq);
> +dilate_time(muldiv64(ti.QuadPart, NANOSECONDS_PER_SECOND,
> clock_freq));
>

Missing "return".

Paolo

 }
>
>  #else
> @@ -838,10 +848,10 @@ static inline int64_t get_clock(void)
>  if (use_rt_clock) {
>  struct timespec ts;
>  clock_gettime(CLOCK_MONOTONIC, &ts);
> -return ts.tv_sec * 10LL + ts.tv_nsec;
> +return dilate_time(ts.tv_sec * 10LL + ts.tv_nsec);
>  } else {
>  /* XXX: using gettimeofday leads to problems if the date
> -   changes, so it should be avoided. */
> +   changes, so it should be avoided. Time is already dilated. */
>  return get_clock_realtime();
>  }
>  }
> diff --git a/util/qemu-timer-common.c b/util/qemu-timer-common.c
> index cc1326f7264..d8895aaccad 100644
> --- a/util/qemu-timer-common.c
> +++ b/util/qemu-timer-common.c
> @@ -28,6 +28,7 @@
>  /* real time host monotonic timer */
>
>  int64_t clock_start;
> +double clock_time_dilation;
>
>  #ifdef _WIN32
>
> --
> 2.47.2
>
>

[PATCH 1/2] qemu/timer: introduce time dilation factor

[Pierrick Bouvier]

This factor is applied to time spent since we read clock for the first
time. It impacts value returned by get_clock() and get_clock_realtime().

Signed-off-by: Pierrick Bouvier 
---
 include/qemu/timer.h | 22 --
 util/qemu-timer-common.c |  1 +
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index abd2204f3be..9c3b8e5506d 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -801,6 +801,18 @@ static inline int64_t get_max_clock_jump(void)
 return 60 * NANOSECONDS_PER_SECOND;
 }
 
+extern int64_t clock_start;
+extern double clock_time_dilation;
+
+static inline int64_t dilate_time(int64_t now)
+{
+g_assert(now >= clock_start);
+if (!clock_time_dilation) {
+return now;
+}
+return clock_start + (now - clock_start) * clock_time_dilation;
+}
+
 /*
  * Low level clock functions
  */
@@ -811,11 +823,9 @@ static inline int64_t get_clock_realtime(void)
 struct timeval tv;
 
 gettimeofday(&tv, NULL);
-return tv.tv_sec * 10LL + (tv.tv_usec * 1000);
+return dilate_time(tv.tv_sec * 10LL + (tv.tv_usec * 1000));
 }
 
-extern int64_t clock_start;
-
 /* Warning: don't insert tracepoints into these functions, they are
also used by simpletrace backend and tracepoints would cause
an infinite recursion! */
@@ -826,7 +836,7 @@ static inline int64_t get_clock(void)
 {
 LARGE_INTEGER ti;
 QueryPerformanceCounter(&ti);
-return muldiv64(ti.QuadPart, NANOSECONDS_PER_SECOND, clock_freq);
+dilate_time(muldiv64(ti.QuadPart, NANOSECONDS_PER_SECOND, clock_freq));
 }
 
 #else
@@ -838,10 +848,10 @@ static inline int64_t get_clock(void)
 if (use_rt_clock) {
 struct timespec ts;
 clock_gettime(CLOCK_MONOTONIC, &ts);
-return ts.tv_sec * 10LL + ts.tv_nsec;
+return dilate_time(ts.tv_sec * 10LL + ts.tv_nsec);
 } else {
 /* XXX: using gettimeofday leads to problems if the date
-   changes, so it should be avoided. */
+   changes, so it should be avoided. Time is already dilated. */
 return get_clock_realtime();
 }
 }
diff --git a/util/qemu-timer-common.c b/util/qemu-timer-common.c
index cc1326f7264..d8895aaccad 100644
--- a/util/qemu-timer-common.c
+++ b/util/qemu-timer-common.c
@@ -28,6 +28,7 @@
 /* real time host monotonic timer */
 
 int64_t clock_start;
+double clock_time_dilation;
 
 #ifdef _WIN32
 
-- 
2.47.2