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

[Sebastian Tanase]

----- Mail original -----
> De: "Frederic Konrad" <address@hidden>
> À: "Sebastian Tanase" <address@hidden>, address@hidden
> Cc: address@hidden, "peter maydell" <address@hidden>, address@hidden, 
> address@hidden,
> address@hidden, address@hidden, address@hidden, address@hidden, 
> address@hidden, address@hidden,
> address@hidden, address@hidden, address@hidden, address@hidden, 
> address@hidden, address@hidden
> Envoyé: Mardi 1 Juillet 2014 09:47:37
> Objet: Re: [Qemu-devel] [RFC PATCH V3 6/6] monitor: Add drift info to 'info 
> jit'
> 
> 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 <address@hidden>
> > Tested-by: Camille Bégué <address@hidden>
> > ---
> >   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
> 

Good catch, I'll fix this in V4.

Thanks,

Sebastian
> > +}
> > +
> > +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)
> 
>