Re: [Qemu-devel] [RFC PATCH V3 6/6] monitor: Add drift info to 'info jit'

[Frederic Konrad <fred.konrad@greensocs.com>]

On 30/06/2014 15:59, Sebastian Tanase wrote:
> Show in 'info jit' the current delay between the host clock
> and the guest clock. In addition, print the maximum advance
> and delay of the guest compared to the host.
>
> Signed-off-by: Sebastian Tanase <sebastian.tanase@openwide.fr>
> Tested-by: Camille Bégué <camille.begue@openwide.fr>
> ---
>   cpu-exec.c            | 61 +++++++++++++++++++++++++++++++++++----------------
>   cpus.c                | 17 ++++++++++++++
>   include/qemu-common.h |  5 +++++
>   monitor.c             |  1 +
>   4 files changed, 65 insertions(+), 19 deletions(-)
>
> diff --git a/cpu-exec.c b/cpu-exec.c
> index 4a4533d..06809f2 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -104,25 +104,18 @@ static void init_delay_params(SyncClocks *sc,
>                                 int64_t realtime_clock_value,
>                                 const CPUState *cpu)
>   {
> -    static int64_t clocks_offset = -1;
> -    int64_t virtual_clock_value;
>       if (!icount_align_option) {
>           return;
>       }
> -    /* On x86 target architecture, the PIT reset function (called
> -       by qemu_system_reset) will end up calling qemu_clock_warp
> -       and then icount_warp_rt changing vm_clock_warp_start from 0 (initial
> -       value) to -1. This in turn will make us skip the initial offset
> -       between the real and virtual clocks (initially virtual clock is 0).
> -       Therefore we impose that the first time we run the cpu
> -       the host and virtual clocks should be aligned; we don't alter any of
> -       the clocks, we just calculate the difference between them. */
> -    virtual_clock_value = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> -    if (clocks_offset == -1) {
> -        clocks_offset = realtime_clock_value - virtual_clock_value;
> -    }
> -    sc->diff_clk = virtual_clock_value - realtime_clock_value + clocks_offset;
> +    sc->diff_clk = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) -
> +                   realtime_clock_value + clocks_offset;
>       sc->original_instr_counter = cpu->icount_extra + cpu->icount_decr.u16.low;
> +    if (sc->diff_clk < max_delay) {
> +        max_delay = sc->diff_clk;
> +    }
> +    if (sc->diff_clk > max_advance) {
> +        max_advance = sc->diff_clk;
> +    }
>   }
>   static void print_delay(InformDelay *indl, int64_t diff_clk)
>   {
> @@ -160,10 +153,32 @@ static void init_inform(InformDelay *indl, int64_t realtime_clock_value)
>   
>   static void compute_value_of_rtc(int64_t *realtime_clock_value)
>   {
> -    if (!icount_align_option) {
> -        return;
> +    /* When using align, we use every time the value of the host clock
> +       whereas when not using align, we only need it once to calculate
> +       the offset between the host and virtual clocks. We then use this
> +       value to correctly print the delay between the 2 clocks when using
> +       "info jit" in the monitor. */
> +    if (icount_align_option) {
> +        *realtime_clock_value = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
> +    } else if (*realtime_clock_value == 0) {
> +        *realtime_clock_value = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
> +    }
Why not "||" both if as they finally do the same thing?

Fred

> +}
> +
> +static void compute_clocks_offset(int64_t realtime_clock_value)
> +{
> +    /* On x86 target architecture, the PIT reset function (called
> +       by qemu_system_reset) will end up calling qemu_clock_warp
> +       and then icount_warp_rt changing vm_clock_warp_start from 0 (initial
> +       value) to -1. This in turn will make us skip the initial offset
> +       between the real and virtual clocks (initially virtual clock is 0).
> +       Therefore we suppose that the first time we run the cpu
> +       the host and virtual clocks should be aligned; we don't alter any of
> +       the clocks, we just calculate the difference between them. */
> +    if (clocks_offset == -1) {
> +        clocks_offset = realtime_clock_value -
> +                        qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>       }
> -    *realtime_clock_value = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
>   }
>   #else
>   /* We don't use the align feature for User emulation
> @@ -189,6 +204,10 @@ static void init_inform(InformDelay *indl, int64_t realtime_clock_value)
>   static void compute_value_of_rtc(int64_t *realtime_clock_value)
>   {
>   }
> +
> +static void compute_clocks_offset(int64_t realtime_clock_value)
> +{
> +}
>   #endif /* CONFIG USER ONLY */
>   
>   void cpu_loop_exit(CPUState *cpu)
> @@ -396,7 +415,7 @@ int cpu_exec(CPUArchState *env)
>       uint8_t *tc_ptr;
>       uintptr_t next_tb;
>       /* Delay algorithm */
> -    int64_t realtime_clock_value;
> +    static int64_t realtime_clock_value;
>       static SyncClocks sc = {
>           .init_delay = init_delay_params,
>           .perform_align = align_clocks
> @@ -465,6 +484,10 @@ int cpu_exec(CPUArchState *env)
>       /* Calculating the realtime is expensive so we do it once here
>          and then pass this value around. */
>       compute_value_of_rtc(&realtime_clock_value);
> +    /* We calculate the clocks_offset here, the very first time
> +       we run the cpu; we do it here because it gives us the best
> +       accuracy. */
> +    compute_clocks_offset(realtime_clock_value);
>       /* Calculate difference between guest clock and host clock.
>          This delay includes the delay of the last cycle, so
>          what we have to do is sleep until it is 0. As for the
> diff --git a/cpus.c b/cpus.c
> index 312c2ee..9947d40 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -64,6 +64,9 @@
>   #endif /* CONFIG_LINUX */
>   
>   static CPUState *next_cpu;
> +int64_t clocks_offset = -1;
> +int64_t max_delay;
> +int64_t max_advance;
>   
>   bool cpu_is_stopped(CPUState *cpu)
>   {
> @@ -1512,3 +1515,17 @@ void qmp_inject_nmi(Error **errp)
>       error_set(errp, QERR_UNSUPPORTED);
>   #endif
>   }
> +
> +void dump_drift_info(FILE *f, fprintf_function cpu_fprintf)
> +{
> +    cpu_fprintf(f, "Host - Guest clock  %ld(ms)\n",
> +                (qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - clocks_offset -
> +                 qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL))/SCALE_MS);
> +    if (icount_align_option) {
> +        cpu_fprintf(f, "Max guest delay     %ld(ms)\n", -max_delay/SCALE_MS);
> +        cpu_fprintf(f, "Max guest advance   %ld(ms)\n", max_advance/SCALE_MS);
> +    } else {
> +        cpu_fprintf(f, "Max guest delay     NA\n");
> +        cpu_fprintf(f, "Max guest advance   NA\n");
> +    }
> +}
> diff --git a/include/qemu-common.h b/include/qemu-common.h
> index f568635..f86b27b 100644
> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -110,6 +110,11 @@ void configure_icount(QemuOpts *opts, Error **errp);
>   extern int use_icount;
>   extern int icount_time_shift;
>   extern int icount_align_option;
> +extern int64_t clocks_offset;
> +/* drift information for info jit command */
> +extern int64_t max_delay;
> +extern int64_t max_advance;
> +void dump_drift_info(FILE *f, fprintf_function cpu_fprintf);
>   
>   #include "qemu/osdep.h"
>   #include "qemu/bswap.h"
> diff --git a/monitor.c b/monitor.c
> index 799131b..23c5534 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1047,6 +1047,7 @@ static void do_info_registers(Monitor *mon, const QDict *qdict)
>   static void do_info_jit(Monitor *mon, const QDict *qdict)
>   {
>       dump_exec_info((FILE *)mon, monitor_fprintf);
> +    dump_drift_info((FILE *)mon, monitor_fprintf);
>   }
>   
>   static void do_info_history(Monitor *mon, const QDict *qdict)