| Message ID | 20250610212829.2818792-2-pierrick.bouvier@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | control guest time using a dilation factor | expand |
Il mar 10 giu 2025, 23:28 Pierrick Bouvier <pierrick.bouvier@linaro.org> 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 * 1000000000LL + (tv.tv_usec * 1000); > + return dilate_time(tv.tv_sec * 1000000000LL + (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 * 1000000000LL + ts.tv_nsec; > + return dilate_time(ts.tv_sec * 1000000000LL + 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 > >
On 6/11/25 12:22 AM, Paolo Bonzini wrote: > > > Il mar 10 giu 2025, 23:28 Pierrick Bouvier <pierrick.bouvier@linaro.org > <mailto:pierrick.bouvier@linaro.org>> 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 * 1000000000LL + (tv.tv_usec * 1000); > + return dilate_time(tv.tv_sec * 1000000000LL + (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 * 1000000000LL + ts.tv_nsec; > + return dilate_time(ts.tv_sec * 1000000000LL + 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
Il mer 11 giu 2025, 20:55 Pierrick Bouvier <pierrick.bouvier@linaro.org> 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
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 * 1000000000LL + (tv.tv_usec * 1000); + return dilate_time(tv.tv_sec * 1000000000LL + (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 * 1000000000LL + ts.tv_nsec; + return dilate_time(ts.tv_sec * 1000000000LL + 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
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 <pierrick.bouvier@linaro.org> --- include/qemu/timer.h | 22 ++++++++++++++++------ util/qemu-timer-common.c | 1 + 2 files changed, 17 insertions(+), 6 deletions(-)