[Qemu-devel] [RFC PATCH V3 0/6] icount: Implement delay algorithm between guest and host clocks

[Qemu-devel] [RFC PATCH V3 0/6] icount: Implement delay algorithm between guest and host clocks

[Sebastian Tanase]

The icount option already implemented in QEMU allows the guest to run at a theoretical
frequency of 1/(2^N) GHz (N is the icount parameter). The goal of this patch is to have a
real guest frequency close to the one imposed by using the icount option.

The main idea behind the algorithm is that we compare the virtual monotonic clock and the
host monotonic clock. For big icounts (on our test machine, an i5 CPU @ 3.10GHz, icounts
starting at 6) the guest clock will be ahead of the host clock. In this case, we try to
sleep QEMU for the difference between the 2 clocks. Therefore, the guest would have
executed for a period almost equally to the one imposed by icount. We should point out
that the algorithm works only for those icounts that allow the guest clock to be in front
of the host clock.

The first patch adds support for QemuOpts for the 'icount' parameter. It also adds a
suboption called 'shift' that will hold the value for 'icount'. Therefore we now have
-icount shift=N|auto or -icount N|auto.

The second patch adds the 'align' suboption for icount.

The third patch exports 'icount_time_shift' so that it can be used in places other than
cpus.c; we need it in cpu-exec.c for calculating for how long we want QEMU to sleep.

The forth patch implements the algorithm used for calculating the delay we want to sleep.
It uses the number of instructions executed by the virtual cpu and also the icount_time_shift.

The fifth patch prints to the console whenever the guest clock runs behind the host
clock. The fastest printing speed is every 2 seconds, and we only print if the align option
is enabled. We also have a limit to 100 printed messages.

The sixth patch adds information about the difference between the host and guest clocks
(taking into account the offset) in the 'info jit' command. We also print the maximum
delay and advance of the guest clock compared to the host clock.

v2 -> v3

* Split previous first patch into 2 patches.
* Use error_abort to handle aborting on error (as suggested by Paolo Bonzini).
* Change delay algorithm to use structures and function pointers in order to
  avoid !defined(CONFIG_USER_ONLY) (as suggested by Paolo Bonzini).
* Limit the number of printed messages and add drift information in the
  'info jit' command (as suggested by Paolo Bonzini).

Sebastian Tanase (6):
  icount: Add QemuOpts for icount
  icount: Add align option to icount
  icount: Make icount_time_shift available everywhere
  cpu_exec: Add sleeping algorithm
  cpu_exec: Print to console if the guest is late
  monitor: Add drift info to 'info jit'

 cpu-exec.c            | 221 ++++++++++++++++++++++++++++++++++++++++++++++++++
 cpus.c                |  48 ++++++++++-
 include/qemu-common.h |  10 ++-
 monitor.c             |   1 +
 qemu-options.hx       |  17 +++-
 qtest.c               |  13 ++-
 vl.c                  |  39 +++++++--
 7 files changed, 333 insertions(+), 16 deletions(-)

-- 
2.0.0.rc2

[Qemu-devel] [RFC PATCH V3 1/6] icount: Add QemuOpts for icount

[Sebastian Tanase]

Make icount parameter use QemuOpts style options in order
to easily add other suboptions.

Signed-off-by: Sebastian Tanase <sebastian.tanase@openwide.fr>
Tested-by: Camille Bégué <camille.begue@openwide.fr>
---
 cpus.c                | 10 +++++++++-
 include/qemu-common.h |  3 ++-
 qemu-options.hx       |  4 ++--
 qtest.c               | 13 +++++++++++--
 vl.c                  | 35 ++++++++++++++++++++++++++++-------
 5 files changed, 52 insertions(+), 13 deletions(-)

diff --git a/cpus.c b/cpus.c
index 5e7f2cf..dcca96a 100644
--- a/cpus.c
+++ b/cpus.c
@@ -440,13 +440,21 @@ static const VMStateDescription vmstate_timers = {
     }
 };
 
-void configure_icount(const char *option)
+void configure_icount(QemuOpts *opts, Error **errp)
 {
+    const char *option;
+
     seqlock_init(&timers_state.vm_clock_seqlock, NULL);
     vmstate_register(NULL, 0, &vmstate_timers, &timers_state);
+    option = qemu_opt_get(opts, "shift");
     if (!option) {
         return;
     }
+    /* When using -icount shift, the shift option will be
+       misinterpreted as a boolean */
+    if (strcmp(option, "on") == 0 || strcmp(option, "off") == 0) {
+        error_setg(errp, "The shift option must be a number or auto");
+    }
 
     icount_warp_timer = timer_new_ns(QEMU_CLOCK_REALTIME,
                                           icount_warp_rt, NULL);
diff --git a/include/qemu-common.h b/include/qemu-common.h
index ae76197..cc346ec 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -41,6 +41,7 @@
 #include <assert.h>
 #include <signal.h>
 #include "glib-compat.h"
+#include "qemu/option.h"
 
 #ifdef _WIN32
 #include "sysemu/os-win32.h"
@@ -105,7 +106,7 @@ static inline char *realpath(const char *path, char *resolved_path)
 #endif
 
 /* icount */
-void configure_icount(const char *option);
+void configure_icount(QemuOpts *opts, Error **errp);
 extern int use_icount;
 
 #include "qemu/osdep.h"
diff --git a/qemu-options.hx b/qemu-options.hx
index 9e54686..143def4 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3011,11 +3011,11 @@ re-inject them.
 ETEXI
 
 DEF("icount", HAS_ARG, QEMU_OPTION_icount, \
-    "-icount [N|auto]\n" \
+    "-icount [shift=N|auto]\n" \
     "                enable virtual instruction counter with 2^N clock ticks per\n" \
     "                instruction\n", QEMU_ARCH_ALL)
 STEXI
-@item -icount [@var{N}|auto]
+@item -icount [shift=@var{N}|auto]
 @findex -icount
 Enable virtual instruction counter.  The virtual cpu will execute one
 instruction every 2^@var{N} ns of virtual time.  If @code{auto} is specified
diff --git a/qtest.c b/qtest.c
index 04a6dc1..ef0d991 100644
--- a/qtest.c
+++ b/qtest.c
@@ -19,6 +19,9 @@
 #include "hw/irq.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/cpus.h"
+#include "qemu/config-file.h"
+#include "qemu/option.h"
+#include "qemu/error-report.h"
 
 #define MAX_IRQ 256
 
@@ -509,10 +512,16 @@ static void qtest_event(void *opaque, int event)
     }
 }
 
-int qtest_init_accel(MachineClass *mc)
+static void configure_qtest_icount(const char *options)
 {
-    configure_icount("0");
+    QemuOpts *opts  = qemu_opts_parse(qemu_find_opts("icount"), options, 1);
+    configure_icount(opts, &error_abort);
+    qemu_opts_del(opts);
+}
 
+int qtest_init_accel(MachineClass *mc)
+{
+    configure_qtest_icount("0");
     return 0;
 }
 
diff --git a/vl.c b/vl.c
index 41ddcd2..103027f 100644
--- a/vl.c
+++ b/vl.c
@@ -537,6 +537,20 @@ static QemuOptsList qemu_mem_opts = {
     },
 };
 
+static QemuOptsList qemu_icount_opts = {
+    .name = "icount",
+    .implied_opt_name = "shift",
+    .merge_lists = true,
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_icount_opts.head),
+    .desc = {
+        {
+            .name = "shift",
+            .type = QEMU_OPT_STRING,
+        },
+        { /* end of list */ }
+    },
+};
+
 /**
  * Get machine options
  *
@@ -2896,13 +2910,12 @@ int main(int argc, char **argv, char **envp)
 {
     int i;
     int snapshot, linux_boot;
-    const char *icount_option = NULL;
     const char *initrd_filename;
     const char *kernel_filename, *kernel_cmdline;
     const char *boot_order;
     DisplayState *ds;
     int cyls, heads, secs, translation;
-    QemuOpts *hda_opts = NULL, *opts, *machine_opts;
+    QemuOpts *hda_opts = NULL, *opts, *machine_opts, *icount_opts = NULL;
     QemuOptsList *olist;
     int optind;
     const char *optarg;
@@ -2967,6 +2980,7 @@ int main(int argc, char **argv, char **envp)
     qemu_add_opts(&qemu_msg_opts);
     qemu_add_opts(&qemu_name_opts);
     qemu_add_opts(&qemu_numa_opts);
+    qemu_add_opts(&qemu_icount_opts);
 
     runstate_init();
 
@@ -3817,7 +3831,11 @@ int main(int argc, char **argv, char **envp)
                 }
                 break;
             case QEMU_OPTION_icount:
-                icount_option = optarg;
+                icount_opts = qemu_opts_parse(qemu_find_opts("icount"),
+                                              optarg, 1);
+                if (!icount_opts) {
+                    exit(1);
+                }
                 break;
             case QEMU_OPTION_incoming:
                 incoming = optarg;
@@ -4294,11 +4312,14 @@ int main(int argc, char **argv, char **envp)
     qemu_spice_init();
 #endif
 
-    if (icount_option && (kvm_enabled() || xen_enabled())) {
-        fprintf(stderr, "-icount is not allowed with kvm or xen\n");
-        exit(1);
+    if (icount_opts) {
+        if (kvm_enabled() || xen_enabled()) {
+            fprintf(stderr, "-icount is not allowed with kvm or xen\n");
+            exit(1);
+        }
+        configure_icount(icount_opts, &error_abort);
+        qemu_opts_del(icount_opts);
     }
-    configure_icount(icount_option);
 
     /* clean up network at qemu process termination */
     atexit(&net_cleanup);
-- 
2.0.0.rc2

[Qemu-devel] [RFC PATCH V3 2/6] icount: Add align option to icount

[Sebastian Tanase]

The align option is used for activating the align algorithm
in order to synchronise the host clock and the guest clock.

Signed-off-by: Sebastian Tanase <sebastian.tanase@openwide.fr>
Tested-by: Camille Bégué <camille.begue@openwide.fr>
---
 cpus.c                | 13 +++++++++++++
 include/qemu-common.h |  1 +
 qemu-options.hx       | 15 +++++++++++++--
 vl.c                  |  4 ++++
 4 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/cpus.c b/cpus.c
index dcca96a..3fa8ce7 100644
--- a/cpus.c
+++ b/cpus.c
@@ -448,13 +448,24 @@ void configure_icount(QemuOpts *opts, Error **errp)
     vmstate_register(NULL, 0, &vmstate_timers, &timers_state);
     option = qemu_opt_get(opts, "shift");
     if (!option) {
+        if (qemu_opt_get(opts, "align") != NULL) {
+            error_setg(errp, "Please specify shift option when using align");
+        }
         return;
     }
+    icount_align_option = qemu_opt_get_bool(opts, "align", false);
     /* When using -icount shift, the shift option will be
        misinterpreted as a boolean */
     if (strcmp(option, "on") == 0 || strcmp(option, "off") == 0) {
         error_setg(errp, "The shift option must be a number or auto");
     }
+    /* When using icount [shift=]N|auto -icount align, shift becomes
+       align therefore we have to specify align=on|off.
+       We do however inform the user whenever the case. */
+    if (strcmp(option, "align") == 0) {
+        error_setg(errp, "Please specify align=on or off so as not "
+         "to create confusion between the shift and align options");
+    }
 
     icount_warp_timer = timer_new_ns(QEMU_CLOCK_REALTIME,
                                           icount_warp_rt, NULL);
@@ -462,6 +473,8 @@ void configure_icount(QemuOpts *opts, Error **errp)
         icount_time_shift = strtol(option, NULL, 0);
         use_icount = 1;
         return;
+    } else if (icount_align_option) {
+        error_setg(errp, "shift=auto and align=on are incompatible");
     }
 
     use_icount = 2;
diff --git a/include/qemu-common.h b/include/qemu-common.h
index cc346ec..860bb15 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -108,6 +108,7 @@ static inline char *realpath(const char *path, char *resolved_path)
 /* icount */
 void configure_icount(QemuOpts *opts, Error **errp);
 extern int use_icount;
+extern int icount_align_option;
 
 #include "qemu/osdep.h"
 #include "qemu/bswap.h"
diff --git a/qemu-options.hx b/qemu-options.hx
index 143def4..379d932 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3011,9 +3011,9 @@ re-inject them.
 ETEXI
 
 DEF("icount", HAS_ARG, QEMU_OPTION_icount, \
-    "-icount [shift=N|auto]\n" \
+    "-icount [shift=N|auto][,align=on|off]\n" \
     "                enable virtual instruction counter with 2^N clock ticks per\n" \
-    "                instruction\n", QEMU_ARCH_ALL)
+    "                instruction and enable aligning the host and virtual clocks\n", QEMU_ARCH_ALL)
 STEXI
 @item -icount [shift=@var{N}|auto]
 @findex -icount
@@ -3026,6 +3026,17 @@ Note that while this option can give deterministic behavior, it does not
 provide cycle accurate emulation.  Modern CPUs contain superscalar out of
 order cores with complex cache hierarchies.  The number of instructions
 executed often has little or no correlation with actual performance.
+
+@option{align=on} will activate the delay algorithm which will try to
+to synchronise the host clock and the virtual clock. The goal is to
+have a guest running at the real frequency imposed by the shift option.
+Whenever the guest clock is behind the host clock and if
+@option{align=on} is specified then we print a messsage to the user
+to inform about the delay.
+Currently this option does not work when @option{shift} is @code{auto}.
+Note: The sync algorithm will work for those shift values for which
+the guest clock runs ahead of the host clock. Typically this happens
+when the shift value is high (how high depends on the host machine).
 ETEXI
 
 DEF("watchdog", HAS_ARG, QEMU_OPTION_watchdog, \
diff --git a/vl.c b/vl.c
index 103027f..d270070 100644
--- a/vl.c
+++ b/vl.c
@@ -183,6 +183,7 @@ uint8_t *boot_splash_filedata;
 size_t boot_splash_filedata_size;
 uint8_t qemu_extra_params_fw[2];
 
+int icount_align_option;
 typedef struct FWBootEntry FWBootEntry;
 
 struct FWBootEntry {
@@ -546,6 +547,9 @@ static QemuOptsList qemu_icount_opts = {
         {
             .name = "shift",
             .type = QEMU_OPT_STRING,
+        }, {
+            .name = "align",
+            .type = QEMU_OPT_BOOL,
         },
         { /* end of list */ }
     },
-- 
2.0.0.rc2

[Qemu-devel] [RFC PATCH V3 3/6] icount: Make icount_time_shift available everywhere

[Sebastian Tanase]

icount_time_shift is used for calculting the delay
qemu has to sleep in order to synchronise the
host and guest clocks. Therefore, we need it in
cpu-exec.c.

Signed-off-by: Sebastian Tanase <sebastian.tanase@openwide.fr>
Tested-by: Camille Bégué <camille.begue@openwide.fr>
---
 cpus.c                | 8 ++++++--
 include/qemu-common.h | 1 +
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/cpus.c b/cpus.c
index 3fa8ce7..312c2ee 100644
--- a/cpus.c
+++ b/cpus.c
@@ -105,8 +105,12 @@ static bool all_cpu_threads_idle(void)
 /* Compensate for varying guest execution speed.  */
 static int64_t qemu_icount_bias;
 static int64_t vm_clock_warp_start;
-/* Conversion factor from emulated instructions to virtual clock ticks.  */
-static int icount_time_shift;
+/* Conversion factor from emulated instructions to virtual clock ticks.
+ * icount_time_shift is defined as extern in include/qemu-common.h because
+ * it is used (in cpu-exec.c) for calculating the delay for sleeping
+ * qemu in order to align the host and virtual clocks.
+ */
+int icount_time_shift;
 /* Arbitrarily pick 1MIPS as the minimum allowable speed.  */
 #define MAX_ICOUNT_SHIFT 10
 
diff --git a/include/qemu-common.h b/include/qemu-common.h
index 860bb15..f568635 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -108,6 +108,7 @@ static inline char *realpath(const char *path, char *resolved_path)
 /* icount */
 void configure_icount(QemuOpts *opts, Error **errp);
 extern int use_icount;
+extern int icount_time_shift;
 extern int icount_align_option;
 
 #include "qemu/osdep.h"
-- 
2.0.0.rc2

[Qemu-devel] [RFC PATCH V3 4/6] cpu_exec: Add sleeping algorithm

[Sebastian Tanase]

The goal is to sleep qemu whenever the guest clock
is in advance compared to the host clock (we use
the monotonic clocks). The amount of time to sleep
is calculated in the execution loop in cpu_exec.

At first, we tried to approximate at each for loop the real time elapsed
while searching for a TB (generating or retrieving from cache) and
executing it. We would then approximate the virtual time corresponding
to the number of virtual instructions executed. The difference between
these 2 values would allow us to know if the guest is in advance or delayed.
However, the function used for measuring the real time
(qemu_clock_get_ns(QEMU_CLOCK_REALTIME)) proved to be very expensive.
We had an added overhead of 13% of the total run time.

Therefore, we modified the algorithm and only take into account the
difference between the 2 clocks at the begining of the cpu_exec function.
During the for loop we try to reduce the advance of the guest only by
computing the virtual time elapsed and sleeping if necessary. The overhead
is thus reduced to 3%. Even though this method still has a noticeable
overhead, it no longer is a bottleneck in trying to achieve a better
guest frequency for which the guest clock is faster than the host one.

As for the the alignement of the 2 clocks, with the first algorithm
the guest clock was oscillating between -1 and 1ms compared to the host clock.
Using the second algorithm we notice that the guest is 5ms behind the host, which
is still acceptable for our use case.

The tests where conducted using fio and stress. The host machine in an i5 CPU at
3.10GHz running Debian Jessie (kernel 3.12). The guest machine is an arm versatile-pb
built with buildroot.

Currently, on our test machine, the lowest icount we can achieve that is suitable for
aligning the 2 clocks is 6. However, we observe that the IO tests (using fio) are
slower than the cpu tests (using stress).

Signed-off-by: Sebastian Tanase <sebastian.tanase@openwide.fr>
Tested-by: Camille Bégué <camille.begue@openwide.fr>
---
 cpu-exec.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 112 insertions(+)

diff --git a/cpu-exec.c b/cpu-exec.c
index 38e5f02..ac741b7 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -22,6 +22,102 @@
 #include "tcg.h"
 #include "qemu/atomic.h"
 #include "sysemu/qtest.h"
+#include "qemu/timer.h"
+
+/* Structs and function pointers for delaying the host */
+typedef struct SyncClocks SyncClocks;
+typedef void (*init_delay_func)(SyncClocks *sc,
+                                const CPUState *cpu);
+typedef void (*perform_align_func)(SyncClocks *sc,
+                                   const CPUState *cpu);
+struct SyncClocks {
+    int64_t diff_clk;
+    int64_t original_instr_counter;
+    init_delay_func init_delay;
+    perform_align_func perform_align;
+};
+
+#if !defined(CONFIG_USER_ONLY)
+/* Allow the guest to have a max 3ms advance.
+ * The difference between the 2 clocks could therefore
+ * oscillate around 0.
+ */
+#define VM_CLOCK_ADVANCE 3000000
+
+static int64_t delay_host(int64_t diff_clk)
+{
+    struct timespec sleep_delay, rem_delay;
+    if (diff_clk > VM_CLOCK_ADVANCE) {
+        sleep_delay.tv_sec = diff_clk / 1000000000LL;
+        sleep_delay.tv_nsec = diff_clk % 1000000000LL;
+        if (nanosleep(&sleep_delay, &rem_delay) < 0) {
+            diff_clk -= (sleep_delay.tv_sec - rem_delay.tv_sec) * 1000000000LL;
+            diff_clk -= sleep_delay.tv_nsec - rem_delay.tv_nsec;
+        } else {
+            diff_clk = 0;
+        }
+    }
+    return diff_clk;
+}
+
+static int64_t instr_to_vtime(int64_t instr_counter, const CPUState *cpu)
+{
+    int64_t instr_exec_time;
+    instr_exec_time = instr_counter -
+                      (cpu->icount_extra +
+                       cpu->icount_decr.u16.low);
+    instr_exec_time = instr_exec_time << icount_time_shift;
+
+    return instr_exec_time;
+}
+
+static void align_clocks(SyncClocks *sc, const CPUState *cpu)
+{
+    if (!icount_align_option) {
+        return;
+    }
+    sc->diff_clk += instr_to_vtime(sc->original_instr_counter, cpu);
+    sc->original_instr_counter = cpu->icount_extra + cpu->icount_decr.u16.low;
+    sc->diff_clk = delay_host(sc->diff_clk);
+}
+
+static void init_delay_params(SyncClocks *sc,
+                              const CPUState *cpu)
+{
+    static int64_t clocks_offset = -1;
+    int64_t realtime_clock_value, 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. */
+    realtime_clock_value = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
+    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->original_instr_counter = cpu->icount_extra + cpu->icount_decr.u16.low;
+}
+#else
+/* We don't use the align feature for User emulation
+   thus we add empty functions which shall be ignored
+   by the compiler */
+static void align_clocks(SyncClocks *sc, const CPUState *cpu)
+{
+}
+
+static void init_delay_params(SyncClocks *sc,
+                              const CPUState *cpu)
+{
+}
+#endif /* CONFIG USER ONLY */
 
 void cpu_loop_exit(CPUState *cpu)
 {
@@ -227,6 +323,11 @@ int cpu_exec(CPUArchState *env)
     TranslationBlock *tb;
     uint8_t *tc_ptr;
     uintptr_t next_tb;
+    /* Delay algorithm */
+    static SyncClocks sc = {
+        .init_delay = init_delay_params,
+        .perform_align = align_clocks
+    };
     /* This must be volatile so it is not trashed by longjmp() */
     volatile bool have_tb_lock = false;
 
@@ -283,6 +384,11 @@ int cpu_exec(CPUArchState *env)
 #endif
     cpu->exception_index = -1;
 
+    /* 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
+       advance/delay we gain here, we try to fix it next time. */
+    sc.init_delay(&sc, cpu);
     /* prepare setjmp context for exception handling */
     for(;;) {
         if (sigsetjmp(cpu->jmp_env, 0) == 0) {
@@ -672,6 +778,9 @@ int cpu_exec(CPUArchState *env)
                             if (insns_left > 0) {
                                 /* Execute remaining instructions.  */
                                 cpu_exec_nocache(env, insns_left, tb);
+                               /* Try to align the host and virtual clocks
+                                  if the guest is in advance. */
+                                sc.perform_align(&sc, cpu);
                             }
                             cpu->exception_index = EXCP_INTERRUPT;
                             next_tb = 0;
@@ -684,6 +793,9 @@ int cpu_exec(CPUArchState *env)
                     }
                 }
                 cpu->current_tb = NULL;
+                /* Try to align the host and virtual clocks
+                   if the guest is in advance */
+                sc.perform_align(&sc, cpu);
                 /* reset soft MMU for next block (it can currently
                    only be set by a memory fault) */
             } /* for(;;) */
-- 
2.0.0.rc2

[Qemu-devel] [RFC PATCH V3 5/6] cpu_exec: Print to console if the guest is late

[Sebastian Tanase]

If the align option is enabled, we print to the user whenever
the guest clock is behind the host clock in order for he/she
to have a hint about the actual performance. The maximum
print interval is 2s and we limit the number of messages to 100.
If desired, this can be changed in cpu-exec.c

Signed-off-by: Sebastian Tanase <sebastian.tanase@openwide.fr>
Tested-by: Camille Bégué <camille.begue@openwide.fr>
---
 cpu-exec.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 90 insertions(+), 4 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index ac741b7..4a4533d 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -24,12 +24,21 @@
 #include "sysemu/qtest.h"
 #include "qemu/timer.h"
 
-/* Structs and function pointers for delaying the host */
+/* Structs and function pointers for delaying the host
+   and printing the clock difference between the guest
+   and the host. */
 typedef struct SyncClocks SyncClocks;
+typedef struct InformDelay InformDelay;
 typedef void (*init_delay_func)(SyncClocks *sc,
+                                int64_t realtime_clock_value,
                                 const CPUState *cpu);
 typedef void (*perform_align_func)(SyncClocks *sc,
                                    const CPUState *cpu);
+typedef void (*init_inform_delay_func)(InformDelay *indl,
+                                       int64_t realtime_clock_value);
+typedef void (*perform_print_func)(InformDelay *indl,
+                                   int64_t diff_clk);
+
 struct SyncClocks {
     int64_t diff_clk;
     int64_t original_instr_counter;
@@ -37,12 +46,22 @@ struct SyncClocks {
     perform_align_func perform_align;
 };
 
+struct InformDelay {
+    int64_t realtime_clock;
+    unsigned int nb_prints;
+    init_inform_delay_func init_inform_delay;
+    perform_print_func perform_print;
+};
+
 #if !defined(CONFIG_USER_ONLY)
 /* Allow the guest to have a max 3ms advance.
  * The difference between the 2 clocks could therefore
  * oscillate around 0.
  */
 #define VM_CLOCK_ADVANCE 3000000
+#define THRESHOLD_REDUCE 1.5
+#define MAX_DELAY_PRINT_RATE 2
+#define MAX_NB_PRINTS 100
 
 static int64_t delay_host(int64_t diff_clk)
 {
@@ -82,10 +101,11 @@ static void align_clocks(SyncClocks *sc, const CPUState *cpu)
 }
 
 static void init_delay_params(SyncClocks *sc,
+                              int64_t realtime_clock_value,
                               const CPUState *cpu)
 {
     static int64_t clocks_offset = -1;
-    int64_t realtime_clock_value, virtual_clock_value;
+    int64_t virtual_clock_value;
     if (!icount_align_option) {
         return;
     }
@@ -97,7 +117,6 @@ static void init_delay_params(SyncClocks *sc,
        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. */
-    realtime_clock_value = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
     virtual_clock_value = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
     if (clocks_offset == -1) {
         clocks_offset = realtime_clock_value - virtual_clock_value;
@@ -105,6 +124,47 @@ static void init_delay_params(SyncClocks *sc,
     sc->diff_clk = virtual_clock_value - realtime_clock_value + clocks_offset;
     sc->original_instr_counter = cpu->icount_extra + cpu->icount_decr.u16.low;
 }
+static void print_delay(InformDelay *indl, int64_t diff_clk)
+{
+    static float threshold_delay;
+    static int64_t last_realtime_clock;
+    if (icount_align_option &&
+        (indl->realtime_clock - last_realtime_clock) / 1000000000LL
+        >= MAX_DELAY_PRINT_RATE && indl->nb_prints < MAX_NB_PRINTS) {
+        if (-diff_clk / (float)1000000000LL > threshold_delay) {
+            threshold_delay = (-diff_clk / 1000000000LL) + 1;
+            printf("Warning: The guest is late by %.1f to %.1f seconds\n",
+                   threshold_delay - 1,
+                   threshold_delay);
+            indl->nb_prints++;
+        } else if (-diff_clk / (float)1000000000LL <
+                   (threshold_delay - THRESHOLD_REDUCE)) {
+            threshold_delay = (-diff_clk / 1000000000LL) + 1;
+            printf("Warning: The guest has reduced the delay and is now "
+                   "late by %.1f to %.1f seconds\n",
+                   threshold_delay - 1,
+                   threshold_delay);
+            indl->nb_prints++;
+        }
+        last_realtime_clock = indl->realtime_clock;
+    }
+}
+
+static void init_inform(InformDelay *indl, int64_t realtime_clock_value)
+{
+    if (!icount_align_option) {
+        return;
+    }
+    indl->realtime_clock = realtime_clock_value;
+}
+
+static void compute_value_of_rtc(int64_t *realtime_clock_value)
+{
+    if (!icount_align_option) {
+        return;
+    }
+    *realtime_clock_value = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
+}
 #else
 /* We don't use the align feature for User emulation
    thus we add empty functions which shall be ignored
@@ -114,9 +174,21 @@ static void align_clocks(SyncClocks *sc, const CPUState *cpu)
 }
 
 static void init_delay_params(SyncClocks *sc,
+                              int64_t realtime_clock_value,
                               const CPUState *cpu)
 {
 }
+static void print_delay(InformDelay *indl, int64_t diff_clk)
+{
+}
+
+static void init_inform(InformDelay *indl, int64_t realtime_clock_value)
+{
+}
+
+static void compute_value_of_rtc(int64_t *realtime_clock_value)
+{
+}
 #endif /* CONFIG USER ONLY */
 
 void cpu_loop_exit(CPUState *cpu)
@@ -324,10 +396,16 @@ int cpu_exec(CPUArchState *env)
     uint8_t *tc_ptr;
     uintptr_t next_tb;
     /* Delay algorithm */
+    int64_t realtime_clock_value;
     static SyncClocks sc = {
         .init_delay = init_delay_params,
         .perform_align = align_clocks
     };
+    /* Print delay control */
+    static InformDelay inform_delay = {
+        .init_inform_delay = init_inform,
+        .perform_print = print_delay
+    };
     /* This must be volatile so it is not trashed by longjmp() */
     volatile bool have_tb_lock = false;
 
@@ -384,11 +462,19 @@ int cpu_exec(CPUArchState *env)
 #endif
     cpu->exception_index = -1;
 
+    /* Calculating the realtime is expensive so we do it once here
+       and then pass this value around. */
+    compute_value_of_rtc(&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
        advance/delay we gain here, we try to fix it next time. */
-    sc.init_delay(&sc, cpu);
+    inform_delay.init_inform_delay(&inform_delay, realtime_clock_value);
+    sc.init_delay(&sc, realtime_clock_value, cpu);
+    /* Print every 2s max if the guest is late. We limit the number
+       of printed messages to NB_PRINT_MAX(currently 100) */
+    inform_delay.perform_print(&inform_delay, sc.diff_clk);
+
     /* prepare setjmp context for exception handling */
     for(;;) {
         if (sigsetjmp(cpu->jmp_env, 0) == 0) {
-- 
2.0.0.rc2

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

[Sebastian Tanase]

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);
+    }
+}
+
+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)
-- 
2.0.0.rc2

Re: [Qemu-devel] [RFC PATCH V3 2/6] icount: Add align option to icount

[Paolo Bonzini]

Il 30/06/2014 15:59, Sebastian Tanase ha scritto:
> The align option is used for activating the align algorithm
> in order to synchronise the host clock and the guest clock.
>
> Signed-off-by: Sebastian Tanase <sebastian.tanase@openwide.fr>
> Tested-by: Camille Bégué <camille.begue@openwide.fr>
> ---
>  cpus.c                | 13 +++++++++++++
>  include/qemu-common.h |  1 +
>  qemu-options.hx       | 15 +++++++++++++--
>  vl.c                  |  4 ++++
>  4 files changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/cpus.c b/cpus.c
> index dcca96a..3fa8ce7 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -448,13 +448,24 @@ void configure_icount(QemuOpts *opts, Error **errp)
>      vmstate_register(NULL, 0, &vmstate_timers, &timers_state);
>      option = qemu_opt_get(opts, "shift");
>      if (!option) {
> +        if (qemu_opt_get(opts, "align") != NULL) {
> +            error_setg(errp, "Please specify shift option when using align");
> +        }
>          return;
>      }
> +    icount_align_option = qemu_opt_get_bool(opts, "align", false);
>      /* When using -icount shift, the shift option will be
>         misinterpreted as a boolean */
>      if (strcmp(option, "on") == 0 || strcmp(option, "off") == 0) {
>          error_setg(errp, "The shift option must be a number or auto");
>      }
> +    /* When using icount [shift=]N|auto -icount align, shift becomes
> +       align therefore we have to specify align=on|off.
> +       We do however inform the user whenever the case. */
> +    if (strcmp(option, "align") == 0) {
> +        error_setg(errp, "Please specify align=on or off so as not "
> +         "to create confusion between the shift and align options");
> +    }

Can you prepare a follow-up (on top of this patch) that instead adds 
proper error handling to the strtol call below?  Then these strcmps can 
probably go away.

Paolo

Re: [Qemu-devel] [RFC PATCH V3 4/6] cpu_exec: Add sleeping algorithm

[Paolo Bonzini]

Il 30/06/2014 15:59, Sebastian Tanase ha scritto:
> The goal is to sleep qemu whenever the guest clock
> is in advance compared to the host clock (we use
> the monotonic clocks). The amount of time to sleep
> is calculated in the execution loop in cpu_exec.
>
> At first, we tried to approximate at each for loop the real time elapsed
> while searching for a TB (generating or retrieving from cache) and
> executing it. We would then approximate the virtual time corresponding
> to the number of virtual instructions executed. The difference between
> these 2 values would allow us to know if the guest is in advance or delayed.
> However, the function used for measuring the real time
> (qemu_clock_get_ns(QEMU_CLOCK_REALTIME)) proved to be very expensive.
> We had an added overhead of 13% of the total run time.
>
> Therefore, we modified the algorithm and only take into account the
> difference between the 2 clocks at the begining of the cpu_exec function.
> During the for loop we try to reduce the advance of the guest only by
> computing the virtual time elapsed and sleeping if necessary. The overhead
> is thus reduced to 3%. Even though this method still has a noticeable
> overhead, it no longer is a bottleneck in trying to achieve a better
> guest frequency for which the guest clock is faster than the host one.
>
> As for the the alignement of the 2 clocks, with the first algorithm
> the guest clock was oscillating between -1 and 1ms compared to the host clock.
> Using the second algorithm we notice that the guest is 5ms behind the host, which
> is still acceptable for our use case.
>
> The tests where conducted using fio and stress. The host machine in an i5 CPU at
> 3.10GHz running Debian Jessie (kernel 3.12). The guest machine is an arm versatile-pb
> built with buildroot.
>
> Currently, on our test machine, the lowest icount we can achieve that is suitable for
> aligning the 2 clocks is 6. However, we observe that the IO tests (using fio) are
> slower than the cpu tests (using stress).
>
> Signed-off-by: Sebastian Tanase <sebastian.tanase@openwide.fr>
> Tested-by: Camille Bégué <camille.begue@openwide.fr>
> ---
>  cpu-exec.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 112 insertions(+)
>
> diff --git a/cpu-exec.c b/cpu-exec.c
> index 38e5f02..ac741b7 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -22,6 +22,102 @@
>  #include "tcg.h"
>  #include "qemu/atomic.h"
>  #include "sysemu/qtest.h"
> +#include "qemu/timer.h"
> +
> +/* Structs and function pointers for delaying the host */
> +typedef struct SyncClocks SyncClocks;
> +typedef void (*init_delay_func)(SyncClocks *sc,
> +                                const CPUState *cpu);
> +typedef void (*perform_align_func)(SyncClocks *sc,
> +                                   const CPUState *cpu);
> +struct SyncClocks {
> +    int64_t diff_clk;
> +    int64_t original_instr_counter;
> +    init_delay_func init_delay;
> +    perform_align_func perform_align;
> +};

I don't remember exactly what I had in mind :) but if I remove these 
pointers from your patches, the code already looks nice, with no 
CONFIG_USER_ONLY except just below here.

> +#if !defined(CONFIG_USER_ONLY)
> +/* Allow the guest to have a max 3ms advance.
> + * The difference between the 2 clocks could therefore
> + * oscillate around 0.
> + */
> +#define VM_CLOCK_ADVANCE 3000000

How did you tune this?

> +static int64_t delay_host(int64_t diff_clk)
> +{
> +    struct timespec sleep_delay, rem_delay;
> +    if (diff_clk > VM_CLOCK_ADVANCE) {
> +        sleep_delay.tv_sec = diff_clk / 1000000000LL;
> +        sleep_delay.tv_nsec = diff_clk % 1000000000LL;
> +        if (nanosleep(&sleep_delay, &rem_delay) < 0) {
> +            diff_clk -= (sleep_delay.tv_sec - rem_delay.tv_sec) * 1000000000LL;
> +            diff_clk -= sleep_delay.tv_nsec - rem_delay.tv_nsec;

I just remembered that nanosleep doesn't exist on Windows. :(  The 
rem_delay feature of nanosleep is very useful, and I don't think there 
is an equivalent.  So for now we shall make this POSIX only.

Paolo

> +        } else {
> +            diff_clk = 0;
> +        }
> +    }
> +    return diff_clk;
> +}
> +
> +static int64_t instr_to_vtime(int64_t instr_counter, const CPUState *cpu)
> +{
> +    int64_t instr_exec_time;
> +    instr_exec_time = instr_counter -
> +                      (cpu->icount_extra +
> +                       cpu->icount_decr.u16.low);
> +    instr_exec_time = instr_exec_time << icount_time_shift;
> +
> +    return instr_exec_time;
> +}
> +
> +static void align_clocks(SyncClocks *sc, const CPUState *cpu)
> +{
> +    if (!icount_align_option) {
> +        return;
> +    }
> +    sc->diff_clk += instr_to_vtime(sc->original_instr_counter, cpu);
> +    sc->original_instr_counter = cpu->icount_extra + cpu->icount_decr.u16.low;
> +    sc->diff_clk = delay_host(sc->diff_clk);
> +}
> +
> +static void init_delay_params(SyncClocks *sc,
> +                              const CPUState *cpu)
> +{
> +    static int64_t clocks_offset = -1;
> +    int64_t realtime_clock_value, 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. */
> +    realtime_clock_value = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
> +    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->original_instr_counter = cpu->icount_extra + cpu->icount_decr.u16.low;
> +}
> +#else
> +/* We don't use the align feature for User emulation
> +   thus we add empty functions which shall be ignored
> +   by the compiler */
> +static void align_clocks(SyncClocks *sc, const CPUState *cpu)
> +{
> +}
> +
> +static void init_delay_params(SyncClocks *sc,
> +                              const CPUState *cpu)
> +{
> +}
> +#endif /* CONFIG USER ONLY */
>
>  void cpu_loop_exit(CPUState *cpu)
>  {
> @@ -227,6 +323,11 @@ int cpu_exec(CPUArchState *env)
>      TranslationBlock *tb;
>      uint8_t *tc_ptr;
>      uintptr_t next_tb;
> +    /* Delay algorithm */
> +    static SyncClocks sc = {
> +        .init_delay = init_delay_params,
> +        .perform_align = align_clocks
> +    };
>      /* This must be volatile so it is not trashed by longjmp() */
>      volatile bool have_tb_lock = false;
>
> @@ -283,6 +384,11 @@ int cpu_exec(CPUArchState *env)
>  #endif
>      cpu->exception_index = -1;
>
> +    /* 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
> +       advance/delay we gain here, we try to fix it next time. */
> +    sc.init_delay(&sc, cpu);
>      /* prepare setjmp context for exception handling */
>      for(;;) {
>          if (sigsetjmp(cpu->jmp_env, 0) == 0) {
> @@ -672,6 +778,9 @@ int cpu_exec(CPUArchState *env)
>                              if (insns_left > 0) {
>                                  /* Execute remaining instructions.  */
>                                  cpu_exec_nocache(env, insns_left, tb);
> +                               /* Try to align the host and virtual clocks
> +                                  if the guest is in advance. */
> +                                sc.perform_align(&sc, cpu);
>                              }
>                              cpu->exception_index = EXCP_INTERRUPT;
>                              next_tb = 0;
> @@ -684,6 +793,9 @@ int cpu_exec(CPUArchState *env)
>                      }
>                  }
>                  cpu->current_tb = NULL;
> +                /* Try to align the host and virtual clocks
> +                   if the guest is in advance */
> +                sc.perform_align(&sc, cpu);
>                  /* reset soft MMU for next block (it can currently
>                     only be set by a memory fault) */
>              } /* for(;;) */
>

Re: [Qemu-devel] [RFC PATCH V3 4/6] cpu_exec: Add sleeping algorithm

[Paolo Bonzini]

Il 30/06/2014 15:59, Sebastian Tanase ha scritto:
> The goal is to sleep qemu whenever the guest clock
> is in advance compared to the host clock (we use
> the monotonic clocks). The amount of time to sleep
> is calculated in the execution loop in cpu_exec.
> 
> At first, we tried to approximate at each for loop the real time elapsed
> while searching for a TB (generating or retrieving from cache) and
> executing it. We would then approximate the virtual time corresponding
> to the number of virtual instructions executed. The difference between
> these 2 values would allow us to know if the guest is in advance or delayed.
> However, the function used for measuring the real time
> (qemu_clock_get_ns(QEMU_CLOCK_REALTIME)) proved to be very expensive.
> We had an added overhead of 13% of the total run time.
> 
> Therefore, we modified the algorithm and only take into account the
> difference between the 2 clocks at the begining of the cpu_exec function.
> During the for loop we try to reduce the advance of the guest only by
> computing the virtual time elapsed and sleeping if necessary. The overhead
> is thus reduced to 3%. Even though this method still has a noticeable
> overhead, it no longer is a bottleneck in trying to achieve a better
> guest frequency for which the guest clock is faster than the host one.
> 
> As for the the alignement of the 2 clocks, with the first algorithm
> the guest clock was oscillating between -1 and 1ms compared to the host clock.
> Using the second algorithm we notice that the guest is 5ms behind the host, which
> is still acceptable for our use case.
> 
> The tests where conducted using fio and stress. The host machine in an i5 CPU at
> 3.10GHz running Debian Jessie (kernel 3.12). The guest machine is an arm versatile-pb
> built with buildroot.
> 
> Currently, on our test machine, the lowest icount we can achieve that is suitable for
> aligning the 2 clocks is 6. However, we observe that the IO tests (using fio) are
> slower than the cpu tests (using stress).
> 
> Signed-off-by: Sebastian Tanase <sebastian.tanase@openwide.fr>
> Tested-by: Camille Bégué <camille.begue@openwide.fr>
> ---
>  cpu-exec.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 112 insertions(+)
> 
> diff --git a/cpu-exec.c b/cpu-exec.c
> index 38e5f02..ac741b7 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -22,6 +22,102 @@
>  #include "tcg.h"
>  #include "qemu/atomic.h"
>  #include "sysemu/qtest.h"
> +#include "qemu/timer.h"
> +
> +/* Structs and function pointers for delaying the host */
> +typedef struct SyncClocks SyncClocks;
> +typedef void (*init_delay_func)(SyncClocks *sc,
> +                                const CPUState *cpu);
> +typedef void (*perform_align_func)(SyncClocks *sc,
> +                                   const CPUState *cpu);
> +struct SyncClocks {
> +    int64_t diff_clk;
> +    int64_t original_instr_counter;
> +    init_delay_func init_delay;
> +    perform_align_func perform_align;
> +};
> +
> +#if !defined(CONFIG_USER_ONLY)
> +/* Allow the guest to have a max 3ms advance.
> + * The difference between the 2 clocks could therefore
> + * oscillate around 0.
> + */
> +#define VM_CLOCK_ADVANCE 3000000
> +
> +static int64_t delay_host(int64_t diff_clk)
> +{
> +    struct timespec sleep_delay, rem_delay;
> +    if (diff_clk > VM_CLOCK_ADVANCE) {
> +        sleep_delay.tv_sec = diff_clk / 1000000000LL;
> +        sleep_delay.tv_nsec = diff_clk % 1000000000LL;
> +        if (nanosleep(&sleep_delay, &rem_delay) < 0) {
> +            diff_clk -= (sleep_delay.tv_sec - rem_delay.tv_sec) * 1000000000LL;
> +            diff_clk -= sleep_delay.tv_nsec - rem_delay.tv_nsec;
> +        } else {
> +            diff_clk = 0;
> +        }
> +    }
> +    return diff_clk;
> +}
> +
> +static int64_t instr_to_vtime(int64_t instr_counter, const CPUState *cpu)
> +{
> +    int64_t instr_exec_time;
> +    instr_exec_time = instr_counter -
> +                      (cpu->icount_extra +
> +                       cpu->icount_decr.u16.low);
> +    instr_exec_time = instr_exec_time << icount_time_shift;
> +
> +    return instr_exec_time;
> +}
> +
> +static void align_clocks(SyncClocks *sc, const CPUState *cpu)
> +{
> +    if (!icount_align_option) {
> +        return;
> +    }
> +    sc->diff_clk += instr_to_vtime(sc->original_instr_counter, cpu);
> +    sc->original_instr_counter = cpu->icount_extra + cpu->icount_decr.u16.low;
> +    sc->diff_clk = delay_host(sc->diff_clk);
> +}
> +
> +static void init_delay_params(SyncClocks *sc,
> +                              const CPUState *cpu)
> +{
> +    static int64_t clocks_offset = -1;
> +    int64_t realtime_clock_value, 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. */

I'm not sure if these gory details are really relevant.  The point, I 
think, is basically that the bases of QEMU_CLOCK_REALTIME and 
QEMU_CLOCK_VIRTUAL differ.  QEMU_CLOCK_REALTIME is based at the Unix epoch,
QEMU_CLOCK_VIRTUAL is based at the time QEMU starts.


> +    realtime_clock_value = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
> +    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->original_instr_counter = cpu->icount_extra + cpu->icount_decr.u16.low;
> +}
> +#else
> +/* We don't use the align feature for User emulation
> +   thus we add empty functions which shall be ignored
> +   by the compiler */
> +static void align_clocks(SyncClocks *sc, const CPUState *cpu)
> +{
> +}
> +
> +static void init_delay_params(SyncClocks *sc,
> +                              const CPUState *cpu)
> +{
> +}
> +#endif /* CONFIG USER ONLY */
>  
>  void cpu_loop_exit(CPUState *cpu)
>  {
> @@ -227,6 +323,11 @@ int cpu_exec(CPUArchState *env)
>      TranslationBlock *tb;
>      uint8_t *tc_ptr;
>      uintptr_t next_tb;
> +    /* Delay algorithm */
> +    static SyncClocks sc = {

This need not be static.

> +        .init_delay = init_delay_params,
> +        .perform_align = align_clocks
> +    };
>      /* This must be volatile so it is not trashed by longjmp() */
>      volatile bool have_tb_lock = false;
>  
> @@ -283,6 +384,11 @@ int cpu_exec(CPUArchState *env)
>  #endif
>      cpu->exception_index = -1;
>  
> +    /* 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
> +       advance/delay we gain here, we try to fix it next time. */
> +    sc.init_delay(&sc, cpu);
>      /* prepare setjmp context for exception handling */
>      for(;;) {
>          if (sigsetjmp(cpu->jmp_env, 0) == 0) {
> @@ -672,6 +778,9 @@ int cpu_exec(CPUArchState *env)
>                              if (insns_left > 0) {
>                                  /* Execute remaining instructions.  */
>                                  cpu_exec_nocache(env, insns_left, tb);
> +                               /* Try to align the host and virtual clocks
> +                                  if the guest is in advance. */
> +                                sc.perform_align(&sc, cpu);
>                              }
>                              cpu->exception_index = EXCP_INTERRUPT;
>                              next_tb = 0;
> @@ -684,6 +793,9 @@ int cpu_exec(CPUArchState *env)
>                      }
>                  }
>                  cpu->current_tb = NULL;
> +                /* Try to align the host and virtual clocks
> +                   if the guest is in advance */
> +                sc.perform_align(&sc, cpu);
>                  /* reset soft MMU for next block (it can currently
>                     only be set by a memory fault) */
>              } /* for(;;) */
> 

Re: [Qemu-devel] [RFC PATCH V3 5/6] cpu_exec: Print to console if the guest is late

[Paolo Bonzini]

Il 30/06/2014 15:59, Sebastian Tanase ha scritto:
> If the align option is enabled, we print to the user whenever
> the guest clock is behind the host clock in order for he/she
> to have a hint about the actual performance. The maximum
> print interval is 2s and we limit the number of messages to 100.
> If desired, this can be changed in cpu-exec.c
>
> Signed-off-by: Sebastian Tanase <sebastian.tanase@openwide.fr>
> Tested-by: Camille Bégué <camille.begue@openwide.fr>
> ---
>  cpu-exec.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 90 insertions(+), 4 deletions(-)
>
> diff --git a/cpu-exec.c b/cpu-exec.c
> index ac741b7..4a4533d 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -24,12 +24,21 @@
>  #include "sysemu/qtest.h"
>  #include "qemu/timer.h"
>
> -/* Structs and function pointers for delaying the host */
> +/* Structs and function pointers for delaying the host
> +   and printing the clock difference between the guest
> +   and the host. */
>  typedef struct SyncClocks SyncClocks;
> +typedef struct InformDelay InformDelay;
>  typedef void (*init_delay_func)(SyncClocks *sc,
> +                                int64_t realtime_clock_value,
>                                  const CPUState *cpu);
>  typedef void (*perform_align_func)(SyncClocks *sc,
>                                     const CPUState *cpu);
> +typedef void (*init_inform_delay_func)(InformDelay *indl,
> +                                       int64_t realtime_clock_value);
> +typedef void (*perform_print_func)(InformDelay *indl,
> +                                   int64_t diff_clk);
> +
>  struct SyncClocks {
>      int64_t diff_clk;
>      int64_t original_instr_counter;
> @@ -37,12 +46,22 @@ struct SyncClocks {
>      perform_align_func perform_align;
>  };
>
> +struct InformDelay {
> +    int64_t realtime_clock;
> +    unsigned int nb_prints;
> +    init_inform_delay_func init_inform_delay;
> +    perform_print_func perform_print;
> +};

I think these structs can be unified.

>  #if !defined(CONFIG_USER_ONLY)
>  /* Allow the guest to have a max 3ms advance.
>   * The difference between the 2 clocks could therefore
>   * oscillate around 0.
>   */
>  #define VM_CLOCK_ADVANCE 3000000
> +#define THRESHOLD_REDUCE 1.5
> +#define MAX_DELAY_PRINT_RATE 2
> +#define MAX_NB_PRINTS 100
>
>  static int64_t delay_host(int64_t diff_clk)
>  {
> @@ -82,10 +101,11 @@ static void align_clocks(SyncClocks *sc, const CPUState *cpu)
>  }
>
>  static void init_delay_params(SyncClocks *sc,
> +                              int64_t realtime_clock_value,
>                                const CPUState *cpu)
>  {
>      static int64_t clocks_offset = -1;
> -    int64_t realtime_clock_value, virtual_clock_value;
> +    int64_t virtual_clock_value;
>      if (!icount_align_option) {
>          return;
>      }
> @@ -97,7 +117,6 @@ static void init_delay_params(SyncClocks *sc,
>         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. */
> -    realtime_clock_value = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
>      virtual_clock_value = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>      if (clocks_offset == -1) {
>          clocks_offset = realtime_clock_value - virtual_clock_value;
> @@ -105,6 +124,47 @@ static void init_delay_params(SyncClocks *sc,
>      sc->diff_clk = virtual_clock_value - realtime_clock_value + clocks_offset;
>      sc->original_instr_counter = cpu->icount_extra + cpu->icount_decr.u16.low;
>  }
> +static void print_delay(InformDelay *indl, int64_t diff_clk)
> +{
> +    static float threshold_delay;
> +    static int64_t last_realtime_clock;
> +    if (icount_align_option &&
> +        (indl->realtime_clock - last_realtime_clock) / 1000000000LL
> +        >= MAX_DELAY_PRINT_RATE && indl->nb_prints < MAX_NB_PRINTS) {
> +        if (-diff_clk / (float)1000000000LL > threshold_delay) {
> +            threshold_delay = (-diff_clk / 1000000000LL) + 1;
> +            printf("Warning: The guest is late by %.1f to %.1f seconds\n",
> +                   threshold_delay - 1,
> +                   threshold_delay);
> +            indl->nb_prints++;
> +        } else if (-diff_clk / (float)1000000000LL <
> +                   (threshold_delay - THRESHOLD_REDUCE)) {
> +            threshold_delay = (-diff_clk / 1000000000LL) + 1;
> +            printf("Warning: The guest has reduced the delay and is now "
> +                   "late by %.1f to %.1f seconds\n",
> +                   threshold_delay - 1,
> +                   threshold_delay);
> +            indl->nb_prints++;
> +        }
> +        last_realtime_clock = indl->realtime_clock;
> +    }
> +}
> +
> +static void init_inform(InformDelay *indl, int64_t realtime_clock_value)
> +{
> +    if (!icount_align_option) {
> +        return;
> +    }
> +    indl->realtime_clock = realtime_clock_value;
> +}
> +
> +static void compute_value_of_rtc(int64_t *realtime_clock_value)
> +{
> +    if (!icount_align_option) {
> +        return;
> +    }
> +    *realtime_clock_value = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
> +}
>  #else
>  /* We don't use the align feature for User emulation
>     thus we add empty functions which shall be ignored
> @@ -114,9 +174,21 @@ static void align_clocks(SyncClocks *sc, const CPUState *cpu)
>  }
>
>  static void init_delay_params(SyncClocks *sc,
> +                              int64_t realtime_clock_value,
>                                const CPUState *cpu)
>  {
>  }
> +static void print_delay(InformDelay *indl, int64_t diff_clk)
> +{
> +}
> +
> +static void init_inform(InformDelay *indl, int64_t realtime_clock_value)
> +{
> +}
> +
> +static void compute_value_of_rtc(int64_t *realtime_clock_value)
> +{
> +}
>  #endif /* CONFIG USER ONLY */
>
>  void cpu_loop_exit(CPUState *cpu)
> @@ -324,10 +396,16 @@ int cpu_exec(CPUArchState *env)
>      uint8_t *tc_ptr;
>      uintptr_t next_tb;
>      /* Delay algorithm */
> +    int64_t realtime_clock_value;
>      static SyncClocks sc = {
>          .init_delay = init_delay_params,
>          .perform_align = align_clocks
>      };
> +    /* Print delay control */
> +    static InformDelay inform_delay = {
> +        .init_inform_delay = init_inform,
> +        .perform_print = print_delay
> +    };
>      /* This must be volatile so it is not trashed by longjmp() */
>      volatile bool have_tb_lock = false;
>
> @@ -384,11 +462,19 @@ int cpu_exec(CPUArchState *env)
>  #endif
>      cpu->exception_index = -1;
>
> +    /* Calculating the realtime is expensive so we do it once here
> +       and then pass this value around. */
> +    compute_value_of_rtc(&realtime_clock_value);

Can this (and compute_clocks_offset) all remain part of init_delay_params?

>      /* 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
>         advance/delay we gain here, we try to fix it next time. */
> -    sc.init_delay(&sc, cpu);
> +    inform_delay.init_inform_delay(&inform_delay, realtime_clock_value);
> +    sc.init_delay(&sc, realtime_clock_value, cpu);
> +    /* Print every 2s max if the guest is late. We limit the number
> +       of printed messages to NB_PRINT_MAX(currently 100) */
> +    inform_delay.perform_print(&inform_delay, sc.diff_clk);

This, too.

Paolo

> +
>      /* prepare setjmp context for exception handling */
>      for(;;) {
>          if (sigsetjmp(cpu->jmp_env, 0) == 0) {
>

Re: [Qemu-devel] [RFC PATCH V3 0/6] icount: Implement delay algorithm between guest and host clocks

[Paolo Bonzini]

Il 30/06/2014 15:59, Sebastian Tanase ha scritto:
> The icount option already implemented in QEMU allows the guest to run at a theoretical
> frequency of 1/(2^N) GHz (N is the icount parameter). The goal of this patch is to have a
> real guest frequency close to the one imposed by using the icount option.
>
> The main idea behind the algorithm is that we compare the virtual monotonic clock and the
> host monotonic clock. For big icounts (on our test machine, an i5 CPU @ 3.10GHz, icounts
> starting at 6) the guest clock will be ahead of the host clock. In this case, we try to
> sleep QEMU for the difference between the 2 clocks. Therefore, the guest would have
> executed for a period almost equally to the one imposed by icount. We should point out
> that the algorithm works only for those icounts that allow the guest clock to be in front
> of the host clock.
>
> The first patch adds support for QemuOpts for the 'icount' parameter. It also adds a
> suboption called 'shift' that will hold the value for 'icount'. Therefore we now have
> -icount shift=N|auto or -icount N|auto.
>
> The second patch adds the 'align' suboption for icount.
>
> The third patch exports 'icount_time_shift' so that it can be used in places other than
> cpus.c; we need it in cpu-exec.c for calculating for how long we want QEMU to sleep.
>
> The forth patch implements the algorithm used for calculating the delay we want to sleep.
> It uses the number of instructions executed by the virtual cpu and also the icount_time_shift.
>
> The fifth patch prints to the console whenever the guest clock runs behind the host
> clock. The fastest printing speed is every 2 seconds, and we only print if the align option
> is enabled. We also have a limit to 100 printed messages.
>
> The sixth patch adds information about the difference between the host and guest clocks
> (taking into account the offset) in the 'info jit' command. We also print the maximum
> delay and advance of the guest clock compared to the host clock.

I think the changes I made are too big to get this in 2.1, but we can 
certainly get this very early in 2.2 instead.  I'll shortly push my 
changes to an "icount" branch at github.com/bonzini/qemu.git.  Can you 
look at it and rebase patches 5 and 6 on top (plus the small change I 
asked for in my review of patch 2)?

Thanks,

Paolo

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

[Frederic Konrad]

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)

Re: [Qemu-devel] [RFC PATCH V3 1/6] icount: Add QemuOpts for icount

[Frederic Konrad]

On 30/06/2014 15:59, Sebastian Tanase wrote:
> Make icount parameter use QemuOpts style options in order
> to easily add other suboptions.
>
> Signed-off-by: Sebastian Tanase <sebastian.tanase@openwide.fr>
> Tested-by: Camille Bégué <camille.begue@openwide.fr>
> ---
>   cpus.c                | 10 +++++++++-
>   include/qemu-common.h |  3 ++-
>   qemu-options.hx       |  4 ++--
>   qtest.c               | 13 +++++++++++--
>   vl.c                  | 35 ++++++++++++++++++++++++++++-------
>   5 files changed, 52 insertions(+), 13 deletions(-)
>
> diff --git a/cpus.c b/cpus.c
> index 5e7f2cf..dcca96a 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -440,13 +440,21 @@ static const VMStateDescription vmstate_timers = {
>       }
>   };
>   
> -void configure_icount(const char *option)
> +void configure_icount(QemuOpts *opts, Error **errp)
>   {
> +    const char *option;
> +
>       seqlock_init(&timers_state.vm_clock_seqlock, NULL);
>       vmstate_register(NULL, 0, &vmstate_timers, &timers_state);
> +    option = qemu_opt_get(opts, "shift");
>       if (!option) {
>           return;
>       }
> +    /* When using -icount shift, the shift option will be
> +       misinterpreted as a boolean */
> +    if (strcmp(option, "on") == 0 || strcmp(option, "off") == 0) {
> +        error_setg(errp, "The shift option must be a number or auto");
> +    }
>   
>       icount_warp_timer = timer_new_ns(QEMU_CLOCK_REALTIME,
>                                             icount_warp_rt, NULL);
> diff --git a/include/qemu-common.h b/include/qemu-common.h
> index ae76197..cc346ec 100644
> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -41,6 +41,7 @@
>   #include <assert.h>
>   #include <signal.h>
>   #include "glib-compat.h"
> +#include "qemu/option.h"
>   
>   #ifdef _WIN32
>   #include "sysemu/os-win32.h"
> @@ -105,7 +106,7 @@ static inline char *realpath(const char *path, char *resolved_path)
>   #endif
>   
>   /* icount */
> -void configure_icount(const char *option);
> +void configure_icount(QemuOpts *opts, Error **errp);
>   extern int use_icount;
>   
>   #include "qemu/osdep.h"
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 9e54686..143def4 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3011,11 +3011,11 @@ re-inject them.
>   ETEXI
>   
>   DEF("icount", HAS_ARG, QEMU_OPTION_icount, \
> -    "-icount [N|auto]\n" \
> +    "-icount [shift=N|auto]\n" \
>       "                enable virtual instruction counter with 2^N clock ticks per\n" \
>       "                instruction\n", QEMU_ARCH_ALL)
>   STEXI
> -@item -icount [@var{N}|auto]
> +@item -icount [shift=@var{N}|auto]
>   @findex -icount
>   Enable virtual instruction counter.  The virtual cpu will execute one
>   instruction every 2^@var{N} ns of virtual time.  If @code{auto} is specified
> diff --git a/qtest.c b/qtest.c
> index 04a6dc1..ef0d991 100644
> --- a/qtest.c
> +++ b/qtest.c
> @@ -19,6 +19,9 @@
>   #include "hw/irq.h"
>   #include "sysemu/sysemu.h"
>   #include "sysemu/cpus.h"
> +#include "qemu/config-file.h"
> +#include "qemu/option.h"
> +#include "qemu/error-report.h"
>   
>   #define MAX_IRQ 256
>   
> @@ -509,10 +512,16 @@ static void qtest_event(void *opaque, int event)
>       }
>   }
>   
> -int qtest_init_accel(MachineClass *mc)
> +static void configure_qtest_icount(const char *options)
>   {
> -    configure_icount("0");
> +    QemuOpts *opts  = qemu_opts_parse(qemu_find_opts("icount"), options, 1);
> +    configure_icount(opts, &error_abort);
> +    qemu_opts_del(opts);
> +}
>   
> +int qtest_init_accel(MachineClass *mc)
> +{
> +    configure_qtest_icount("0");
>       return 0;
>   }
>   
> diff --git a/vl.c b/vl.c
> index 41ddcd2..103027f 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -537,6 +537,20 @@ static QemuOptsList qemu_mem_opts = {
>       },
>   };
>   
> +static QemuOptsList qemu_icount_opts = {
> +    .name = "icount",
> +    .implied_opt_name = "shift",
> +    .merge_lists = true,
> +    .head = QTAILQ_HEAD_INITIALIZER(qemu_icount_opts.head),
> +    .desc = {
> +        {
> +            .name = "shift",
> +            .type = QEMU_OPT_STRING,
> +        },
> +        { /* end of list */ }
> +    },
> +};
> +
>   /**
>    * Get machine options
>    *
> @@ -2896,13 +2910,12 @@ int main(int argc, char **argv, char **envp)
>   {
>       int i;
>       int snapshot, linux_boot;
> -    const char *icount_option = NULL;
>       const char *initrd_filename;
>       const char *kernel_filename, *kernel_cmdline;
>       const char *boot_order;
>       DisplayState *ds;
>       int cyls, heads, secs, translation;
> -    QemuOpts *hda_opts = NULL, *opts, *machine_opts;
> +    QemuOpts *hda_opts = NULL, *opts, *machine_opts, *icount_opts = NULL;
>       QemuOptsList *olist;
>       int optind;
>       const char *optarg;
> @@ -2967,6 +2980,7 @@ int main(int argc, char **argv, char **envp)
>       qemu_add_opts(&qemu_msg_opts);
>       qemu_add_opts(&qemu_name_opts);
>       qemu_add_opts(&qemu_numa_opts);
> +    qemu_add_opts(&qemu_icount_opts);
>   
>       runstate_init();
>   
> @@ -3817,7 +3831,11 @@ int main(int argc, char **argv, char **envp)
>                   }
>                   break;
>               case QEMU_OPTION_icount:
> -                icount_option = optarg;
> +                icount_opts = qemu_opts_parse(qemu_find_opts("icount"),
> +                                              optarg, 1);
> +                if (!icount_opts) {
> +                    exit(1);
> +                }
>                   break;
>               case QEMU_OPTION_incoming:
>                   incoming = optarg;
> @@ -4294,11 +4312,14 @@ int main(int argc, char **argv, char **envp)
>       qemu_spice_init();
>   #endif
>   
> -    if (icount_option && (kvm_enabled() || xen_enabled())) {
> -        fprintf(stderr, "-icount is not allowed with kvm or xen\n");
> -        exit(1);
> +    if (icount_opts) {
> +        if (kvm_enabled() || xen_enabled()) {
> +            fprintf(stderr, "-icount is not allowed with kvm or xen\n");
> +            exit(1);
> +        }
> +        configure_icount(icount_opts, &error_abort);
> +        qemu_opts_del(icount_opts);
>       }
> -    configure_icount(icount_option);
>   
>       /* clean up network at qemu process termination */
>       atexit(&net_cleanup);
Looks ok to me ;).

Fred

Re: [Qemu-devel] [RFC PATCH V3 2/6] icount: Add align option to icount

[Sebastian Tanase]

----- Mail original -----
> De: "Paolo Bonzini" <pbonzini@redhat.com>
> À: "Sebastian Tanase" <sebastian.tanase@openwide.fr>, qemu-devel@nongnu.org
> Cc: aliguori@amazon.com, afaerber@suse.de, rth@twiddle.net, "peter maydell" <peter.maydell@linaro.org>,
> michael@walle.cc, alex@alex.org.uk, stefanha@redhat.com, lcapitulino@redhat.com, crobinso@redhat.com,
> armbru@redhat.com, wenchaoqemu@gmail.com, quintela@redhat.com, kwolf@redhat.com, mjt@tls.msk.ru, mst@redhat.com
> Envoyé: Lundi 30 Juin 2014 18:33:38
> Objet: Re: [RFC PATCH V3 2/6] icount: Add align option to icount
> 
> Il 30/06/2014 15:59, Sebastian Tanase ha scritto:
> > The align option is used for activating the align algorithm
> > in order to synchronise the host clock and the guest clock.
> >
> > Signed-off-by: Sebastian Tanase <sebastian.tanase@openwide.fr>
> > Tested-by: Camille Bégué <camille.begue@openwide.fr>
> > ---
> >  cpus.c                | 13 +++++++++++++
> >  include/qemu-common.h |  1 +
> >  qemu-options.hx       | 15 +++++++++++++--
> >  vl.c                  |  4 ++++
> >  4 files changed, 31 insertions(+), 2 deletions(-)
> >
> > diff --git a/cpus.c b/cpus.c
> > index dcca96a..3fa8ce7 100644
> > --- a/cpus.c
> > +++ b/cpus.c
> > @@ -448,13 +448,24 @@ void configure_icount(QemuOpts *opts, Error
> > **errp)
> >      vmstate_register(NULL, 0, &vmstate_timers, &timers_state);
> >      option = qemu_opt_get(opts, "shift");
> >      if (!option) {
> > +        if (qemu_opt_get(opts, "align") != NULL) {
> > +            error_setg(errp, "Please specify shift option when
> > using align");
> > +        }
> >          return;
> >      }
> > +    icount_align_option = qemu_opt_get_bool(opts, "align", false);
> >      /* When using -icount shift, the shift option will be
> >         misinterpreted as a boolean */
> >      if (strcmp(option, "on") == 0 || strcmp(option, "off") == 0) {
> >          error_setg(errp, "The shift option must be a number or
> >          auto");
> >      }
> > +    /* When using icount [shift=]N|auto -icount align, shift
> > becomes
> > +       align therefore we have to specify align=on|off.
> > +       We do however inform the user whenever the case. */
> > +    if (strcmp(option, "align") == 0) {
> > +        error_setg(errp, "Please specify align=on or off so as not
> > "
> > +         "to create confusion between the shift and align
> > options");
> > +    }
> 
> Can you prepare a follow-up (on top of this patch) that instead adds
> proper error handling to the strtol call below?  Then these strcmps
> can
> probably go away.
> 
> Paolo
> 
Sure, no problem.

Thanks for the advice.

Sebastian

Re: [Qemu-devel] [RFC PATCH V3 4/6] cpu_exec: Add sleeping algorithm

[Sebastian Tanase]

----- Mail original -----
> De: "Paolo Bonzini" <pbonzini@redhat.com>
> À: "Sebastian Tanase" <sebastian.tanase@openwide.fr>, qemu-devel@nongnu.org
> Cc: aliguori@amazon.com, afaerber@suse.de, rth@twiddle.net, "peter maydell" <peter.maydell@linaro.org>,
> michael@walle.cc, alex@alex.org.uk, stefanha@redhat.com, lcapitulino@redhat.com, crobinso@redhat.com,
> armbru@redhat.com, wenchaoqemu@gmail.com, quintela@redhat.com, kwolf@redhat.com, mjt@tls.msk.ru, mst@redhat.com
> Envoyé: Lundi 30 Juin 2014 18:46:13
> Objet: Re: [RFC PATCH V3 4/6] cpu_exec: Add sleeping algorithm
> 
> Il 30/06/2014 15:59, Sebastian Tanase ha scritto:
> > The goal is to sleep qemu whenever the guest clock
> > is in advance compared to the host clock (we use
> > the monotonic clocks). The amount of time to sleep
> > is calculated in the execution loop in cpu_exec.
> >
> > At first, we tried to approximate at each for loop the real time
> > elapsed
> > while searching for a TB (generating or retrieving from cache) and
> > executing it. We would then approximate the virtual time
> > corresponding
> > to the number of virtual instructions executed. The difference
> > between
> > these 2 values would allow us to know if the guest is in advance or
> > delayed.
> > However, the function used for measuring the real time
> > (qemu_clock_get_ns(QEMU_CLOCK_REALTIME)) proved to be very
> > expensive.
> > We had an added overhead of 13% of the total run time.
> >
> > Therefore, we modified the algorithm and only take into account the
> > difference between the 2 clocks at the begining of the cpu_exec
> > function.
> > During the for loop we try to reduce the advance of the guest only
> > by
> > computing the virtual time elapsed and sleeping if necessary. The
> > overhead
> > is thus reduced to 3%. Even though this method still has a
> > noticeable
> > overhead, it no longer is a bottleneck in trying to achieve a
> > better
> > guest frequency for which the guest clock is faster than the host
> > one.
> >
> > As for the the alignement of the 2 clocks, with the first algorithm
> > the guest clock was oscillating between -1 and 1ms compared to the
> > host clock.
> > Using the second algorithm we notice that the guest is 5ms behind
> > the host, which
> > is still acceptable for our use case.
> >
> > The tests where conducted using fio and stress. The host machine in
> > an i5 CPU at
> > 3.10GHz running Debian Jessie (kernel 3.12). The guest machine is
> > an arm versatile-pb
> > built with buildroot.
> >
> > Currently, on our test machine, the lowest icount we can achieve
> > that is suitable for
> > aligning the 2 clocks is 6. However, we observe that the IO tests
> > (using fio) are
> > slower than the cpu tests (using stress).
> >
> > Signed-off-by: Sebastian Tanase <sebastian.tanase@openwide.fr>
> > Tested-by: Camille Bégué <camille.begue@openwide.fr>
> > ---
> >  cpu-exec.c | 112
> >  +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 112 insertions(+)
> >
> > diff --git a/cpu-exec.c b/cpu-exec.c
> > index 38e5f02..ac741b7 100644
> > --- a/cpu-exec.c
> > +++ b/cpu-exec.c
> > @@ -22,6 +22,102 @@
> >  #include "tcg.h"
> >  #include "qemu/atomic.h"
> >  #include "sysemu/qtest.h"
> > +#include "qemu/timer.h"
> > +
> > +/* Structs and function pointers for delaying the host */
> > +typedef struct SyncClocks SyncClocks;
> > +typedef void (*init_delay_func)(SyncClocks *sc,
> > +                                const CPUState *cpu);
> > +typedef void (*perform_align_func)(SyncClocks *sc,
> > +                                   const CPUState *cpu);
> > +struct SyncClocks {
> > +    int64_t diff_clk;
> > +    int64_t original_instr_counter;
> > +    init_delay_func init_delay;
> > +    perform_align_func perform_align;
> > +};
> 
> I don't remember exactly what I had in mind :) but if I remove these
> pointers from your patches, the code already looks nice, with no
> CONFIG_USER_ONLY except just below here.
> 

Ok, I will remove the function pointers then :)

> > +#if !defined(CONFIG_USER_ONLY)
> > +/* Allow the guest to have a max 3ms advance.
> > + * The difference between the 2 clocks could therefore
> > + * oscillate around 0.
> > + */
> > +#define VM_CLOCK_ADVANCE 3000000
> 
> How did you tune this?
> 

I computed this value based on the tests run on my machine. Of course,
this value will be different on a different machine running different
tests.

> > +static int64_t delay_host(int64_t diff_clk)
> > +{
> > +    struct timespec sleep_delay, rem_delay;
> > +    if (diff_clk > VM_CLOCK_ADVANCE) {
> > +        sleep_delay.tv_sec = diff_clk / 1000000000LL;
> > +        sleep_delay.tv_nsec = diff_clk % 1000000000LL;
> > +        if (nanosleep(&sleep_delay, &rem_delay) < 0) {
> > +            diff_clk -= (sleep_delay.tv_sec - rem_delay.tv_sec) *
> > 1000000000LL;
> > +            diff_clk -= sleep_delay.tv_nsec - rem_delay.tv_nsec;
> 
> I just remembered that nanosleep doesn't exist on Windows. :(  The
> rem_delay feature of nanosleep is very useful, and I don't think
> there
> is an equivalent.  So for now we shall make this POSIX only.
> 
> Paolo
> 

Should I surround the nanosleep with #ifndef _WIN32 and then add
Sleep for the Windows case ? or just leave out Windows ?

Sebastian


> > +        } else {
> > +            diff_clk = 0;
> > +        }
> > +    }
> > +    return diff_clk;
> > +}
> > +
> > +static int64_t instr_to_vtime(int64_t instr_counter, const
> > CPUState *cpu)
> > +{
> > +    int64_t instr_exec_time;
> > +    instr_exec_time = instr_counter -
> > +                      (cpu->icount_extra +
> > +                       cpu->icount_decr.u16.low);
> > +    instr_exec_time = instr_exec_time << icount_time_shift;
> > +
> > +    return instr_exec_time;
> > +}
> > +
> > +static void align_clocks(SyncClocks *sc, const CPUState *cpu)
> > +{
> > +    if (!icount_align_option) {
> > +        return;
> > +    }
> > +    sc->diff_clk += instr_to_vtime(sc->original_instr_counter,
> > cpu);
> > +    sc->original_instr_counter = cpu->icount_extra +
> > cpu->icount_decr.u16.low;
> > +    sc->diff_clk = delay_host(sc->diff_clk);
> > +}
> > +
> > +static void init_delay_params(SyncClocks *sc,
> > +                              const CPUState *cpu)
> > +{
> > +    static int64_t clocks_offset = -1;
> > +    int64_t realtime_clock_value, 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.
> > */
> > +    realtime_clock_value = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
> > +    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->original_instr_counter = cpu->icount_extra +
> > cpu->icount_decr.u16.low;
> > +}
> > +#else
> > +/* We don't use the align feature for User emulation
> > +   thus we add empty functions which shall be ignored
> > +   by the compiler */
> > +static void align_clocks(SyncClocks *sc, const CPUState *cpu)
> > +{
> > +}
> > +
> > +static void init_delay_params(SyncClocks *sc,
> > +                              const CPUState *cpu)
> > +{
> > +}
> > +#endif /* CONFIG USER ONLY */
> >
> >  void cpu_loop_exit(CPUState *cpu)
> >  {
> > @@ -227,6 +323,11 @@ int cpu_exec(CPUArchState *env)
> >      TranslationBlock *tb;
> >      uint8_t *tc_ptr;
> >      uintptr_t next_tb;
> > +    /* Delay algorithm */
> > +    static SyncClocks sc = {
> > +        .init_delay = init_delay_params,
> > +        .perform_align = align_clocks
> > +    };
> >      /* This must be volatile so it is not trashed by longjmp() */
> >      volatile bool have_tb_lock = false;
> >
> > @@ -283,6 +384,11 @@ int cpu_exec(CPUArchState *env)
> >  #endif
> >      cpu->exception_index = -1;
> >
> > +    /* 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
> > +       advance/delay we gain here, we try to fix it next time. */
> > +    sc.init_delay(&sc, cpu);
> >      /* prepare setjmp context for exception handling */
> >      for(;;) {
> >          if (sigsetjmp(cpu->jmp_env, 0) == 0) {
> > @@ -672,6 +778,9 @@ int cpu_exec(CPUArchState *env)
> >                              if (insns_left > 0) {
> >                                  /* Execute remaining instructions.
> >                                   */
> >                                  cpu_exec_nocache(env, insns_left,
> >                                  tb);
> > +                               /* Try to align the host and
> > virtual clocks
> > +                                  if the guest is in advance. */
> > +                                sc.perform_align(&sc, cpu);
> >                              }
> >                              cpu->exception_index = EXCP_INTERRUPT;
> >                              next_tb = 0;
> > @@ -684,6 +793,9 @@ int cpu_exec(CPUArchState *env)
> >                      }
> >                  }
> >                  cpu->current_tb = NULL;
> > +                /* Try to align the host and virtual clocks
> > +                   if the guest is in advance */
> > +                sc.perform_align(&sc, cpu);
> >                  /* reset soft MMU for next block (it can currently
> >                     only be set by a memory fault) */
> >              } /* for(;;) */
> >
> 
> 

Re: [Qemu-devel] [RFC PATCH V3 5/6] cpu_exec: Print to console if the guest is late

[Sebastian Tanase]

----- Mail original -----
> De: "Paolo Bonzini" <pbonzini@redhat.com>
> À: "Sebastian Tanase" <sebastian.tanase@openwide.fr>, qemu-devel@nongnu.org
> Cc: aliguori@amazon.com, afaerber@suse.de, rth@twiddle.net, "peter maydell" <peter.maydell@linaro.org>,
> michael@walle.cc, alex@alex.org.uk, stefanha@redhat.com, lcapitulino@redhat.com, crobinso@redhat.com,
> armbru@redhat.com, wenchaoqemu@gmail.com, quintela@redhat.com, kwolf@redhat.com, mjt@tls.msk.ru, mst@redhat.com
> Envoyé: Lundi 30 Juin 2014 19:11:41
> Objet: Re: [RFC PATCH V3 5/6] cpu_exec: Print to console if the guest is late
> 
> Il 30/06/2014 15:59, Sebastian Tanase ha scritto:
> > If the align option is enabled, we print to the user whenever
> > the guest clock is behind the host clock in order for he/she
> > to have a hint about the actual performance. The maximum
> > print interval is 2s and we limit the number of messages to 100.
> > If desired, this can be changed in cpu-exec.c
> >
> > Signed-off-by: Sebastian Tanase <sebastian.tanase@openwide.fr>
> > Tested-by: Camille Bégué <camille.begue@openwide.fr>
> > ---
> >  cpu-exec.c | 94
> >  +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
> >  1 file changed, 90 insertions(+), 4 deletions(-)
> >
> > diff --git a/cpu-exec.c b/cpu-exec.c
> > index ac741b7..4a4533d 100644
> > --- a/cpu-exec.c
> > +++ b/cpu-exec.c
> > @@ -24,12 +24,21 @@
> >  #include "sysemu/qtest.h"
> >  #include "qemu/timer.h"
> >
> > -/* Structs and function pointers for delaying the host */
> > +/* Structs and function pointers for delaying the host
> > +   and printing the clock difference between the guest
> > +   and the host. */
> >  typedef struct SyncClocks SyncClocks;
> > +typedef struct InformDelay InformDelay;
> >  typedef void (*init_delay_func)(SyncClocks *sc,
> > +                                int64_t realtime_clock_value,
> >                                  const CPUState *cpu);
> >  typedef void (*perform_align_func)(SyncClocks *sc,
> >                                     const CPUState *cpu);
> > +typedef void (*init_inform_delay_func)(InformDelay *indl,
> > +                                       int64_t
> > realtime_clock_value);
> > +typedef void (*perform_print_func)(InformDelay *indl,
> > +                                   int64_t diff_clk);
> > +
> >  struct SyncClocks {
> >      int64_t diff_clk;
> >      int64_t original_instr_counter;
> > @@ -37,12 +46,22 @@ struct SyncClocks {
> >      perform_align_func perform_align;
> >  };
> >
> > +struct InformDelay {
> > +    int64_t realtime_clock;
> > +    unsigned int nb_prints;
> > +    init_inform_delay_func init_inform_delay;
> > +    perform_print_func perform_print;
> > +};
> 
> I think these structs can be unified.
> 
> >  #if !defined(CONFIG_USER_ONLY)
> >  /* Allow the guest to have a max 3ms advance.
> >   * The difference between the 2 clocks could therefore
> >   * oscillate around 0.
> >   */
> >  #define VM_CLOCK_ADVANCE 3000000
> > +#define THRESHOLD_REDUCE 1.5
> > +#define MAX_DELAY_PRINT_RATE 2
> > +#define MAX_NB_PRINTS 100
> >
> >  static int64_t delay_host(int64_t diff_clk)
> >  {
> > @@ -82,10 +101,11 @@ static void align_clocks(SyncClocks *sc, const
> > CPUState *cpu)
> >  }
> >
> >  static void init_delay_params(SyncClocks *sc,
> > +                              int64_t realtime_clock_value,
> >                                const CPUState *cpu)
> >  {
> >      static int64_t clocks_offset = -1;
> > -    int64_t realtime_clock_value, virtual_clock_value;
> > +    int64_t virtual_clock_value;
> >      if (!icount_align_option) {
> >          return;
> >      }
> > @@ -97,7 +117,6 @@ static void init_delay_params(SyncClocks *sc,
> >         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.
> >         */
> > -    realtime_clock_value = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
> >      virtual_clock_value = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> >      if (clocks_offset == -1) {
> >          clocks_offset = realtime_clock_value -
> >          virtual_clock_value;
> > @@ -105,6 +124,47 @@ static void init_delay_params(SyncClocks *sc,
> >      sc->diff_clk = virtual_clock_value - realtime_clock_value +
> >      clocks_offset;
> >      sc->original_instr_counter = cpu->icount_extra +
> >      cpu->icount_decr.u16.low;
> >  }
> > +static void print_delay(InformDelay *indl, int64_t diff_clk)
> > +{
> > +    static float threshold_delay;
> > +    static int64_t last_realtime_clock;
> > +    if (icount_align_option &&
> > +        (indl->realtime_clock - last_realtime_clock) /
> > 1000000000LL
> > +        >= MAX_DELAY_PRINT_RATE && indl->nb_prints <
> > MAX_NB_PRINTS) {
> > +        if (-diff_clk / (float)1000000000LL > threshold_delay) {
> > +            threshold_delay = (-diff_clk / 1000000000LL) + 1;
> > +            printf("Warning: The guest is late by %.1f to %.1f
> > seconds\n",
> > +                   threshold_delay - 1,
> > +                   threshold_delay);
> > +            indl->nb_prints++;
> > +        } else if (-diff_clk / (float)1000000000LL <
> > +                   (threshold_delay - THRESHOLD_REDUCE)) {
> > +            threshold_delay = (-diff_clk / 1000000000LL) + 1;
> > +            printf("Warning: The guest has reduced the delay and
> > is now "
> > +                   "late by %.1f to %.1f seconds\n",
> > +                   threshold_delay - 1,
> > +                   threshold_delay);
> > +            indl->nb_prints++;
> > +        }
> > +        last_realtime_clock = indl->realtime_clock;
> > +    }
> > +}
> > +
> > +static void init_inform(InformDelay *indl, int64_t
> > realtime_clock_value)
> > +{
> > +    if (!icount_align_option) {
> > +        return;
> > +    }
> > +    indl->realtime_clock = realtime_clock_value;
> > +}
> > +
> > +static void compute_value_of_rtc(int64_t *realtime_clock_value)
> > +{
> > +    if (!icount_align_option) {
> > +        return;
> > +    }
> > +    *realtime_clock_value =
> > qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
> > +}
> >  #else
> >  /* We don't use the align feature for User emulation
> >     thus we add empty functions which shall be ignored
> > @@ -114,9 +174,21 @@ static void align_clocks(SyncClocks *sc, const
> > CPUState *cpu)
> >  }
> >
> >  static void init_delay_params(SyncClocks *sc,
> > +                              int64_t realtime_clock_value,
> >                                const CPUState *cpu)
> >  {
> >  }
> > +static void print_delay(InformDelay *indl, int64_t diff_clk)
> > +{
> > +}
> > +
> > +static void init_inform(InformDelay *indl, int64_t
> > realtime_clock_value)
> > +{
> > +}
> > +
> > +static void compute_value_of_rtc(int64_t *realtime_clock_value)
> > +{
> > +}
> >  #endif /* CONFIG USER ONLY */
> >
> >  void cpu_loop_exit(CPUState *cpu)
> > @@ -324,10 +396,16 @@ int cpu_exec(CPUArchState *env)
> >      uint8_t *tc_ptr;
> >      uintptr_t next_tb;
> >      /* Delay algorithm */
> > +    int64_t realtime_clock_value;
> >      static SyncClocks sc = {
> >          .init_delay = init_delay_params,
> >          .perform_align = align_clocks
> >      };
> > +    /* Print delay control */
> > +    static InformDelay inform_delay = {
> > +        .init_inform_delay = init_inform,
> > +        .perform_print = print_delay
> > +    };
> >      /* This must be volatile so it is not trashed by longjmp() */
> >      volatile bool have_tb_lock = false;
> >
> > @@ -384,11 +462,19 @@ int cpu_exec(CPUArchState *env)
> >  #endif
> >      cpu->exception_index = -1;
> >
> > +    /* Calculating the realtime is expensive so we do it once here
> > +       and then pass this value around. */
> > +    compute_value_of_rtc(&realtime_clock_value);
> 
> Can this (and compute_clocks_offset) all remain part of
> init_delay_params?
> 
> >      /* 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
> >         advance/delay we gain here, we try to fix it next time. */
> > -    sc.init_delay(&sc, cpu);
> > +    inform_delay.init_inform_delay(&inform_delay,
> > realtime_clock_value);
> > +    sc.init_delay(&sc, realtime_clock_value, cpu);
> > +    /* Print every 2s max if the guest is late. We limit the
> > number
> > +       of printed messages to NB_PRINT_MAX(currently 100) */
> > +    inform_delay.perform_print(&inform_delay, sc.diff_clk);
> 
> This, too.
> 
> Paolo
> 

Unifying the 2 structs will certainly make this possible.

Sebastian

> > +
> >      /* prepare setjmp context for exception handling */
> >      for(;;) {
> >          if (sigsetjmp(cpu->jmp_env, 0) == 0) {
> >
> 
> 
> 

Re: [Qemu-devel] [RFC PATCH V3 0/6] icount: Implement delay algorithm between guest and host clocks

[Sebastian Tanase]

----- Mail original -----
> De: "Paolo Bonzini" <pbonzini@redhat.com>
> À: "Sebastian Tanase" <sebastian.tanase@openwide.fr>, qemu-devel@nongnu.org
> Cc: aliguori@amazon.com, afaerber@suse.de, rth@twiddle.net, "peter maydell" <peter.maydell@linaro.org>,
> michael@walle.cc, alex@alex.org.uk, stefanha@redhat.com, lcapitulino@redhat.com, crobinso@redhat.com,
> armbru@redhat.com, wenchaoqemu@gmail.com, quintela@redhat.com, kwolf@redhat.com, mjt@tls.msk.ru, mst@redhat.com
> Envoyé: Lundi 30 Juin 2014 19:16:48
> Objet: Re: [RFC PATCH V3 0/6] icount: Implement delay algorithm between guest and host clocks
> 
> Il 30/06/2014 15:59, Sebastian Tanase ha scritto:
> > The icount option already implemented in QEMU allows the guest to
> > run at a theoretical
> > frequency of 1/(2^N) GHz (N is the icount parameter). The goal of
> > this patch is to have a
> > real guest frequency close to the one imposed by using the icount
> > option.
> >
> > The main idea behind the algorithm is that we compare the virtual
> > monotonic clock and the
> > host monotonic clock. For big icounts (on our test machine, an i5
> > CPU @ 3.10GHz, icounts
> > starting at 6) the guest clock will be ahead of the host clock. In
> > this case, we try to
> > sleep QEMU for the difference between the 2 clocks. Therefore, the
> > guest would have
> > executed for a period almost equally to the one imposed by icount.
> > We should point out
> > that the algorithm works only for those icounts that allow the
> > guest clock to be in front
> > of the host clock.
> >
> > The first patch adds support for QemuOpts for the 'icount'
> > parameter. It also adds a
> > suboption called 'shift' that will hold the value for 'icount'.
> > Therefore we now have
> > -icount shift=N|auto or -icount N|auto.
> >
> > The second patch adds the 'align' suboption for icount.
> >
> > The third patch exports 'icount_time_shift' so that it can be used
> > in places other than
> > cpus.c; we need it in cpu-exec.c for calculating for how long we
> > want QEMU to sleep.
> >
> > The forth patch implements the algorithm used for calculating the
> > delay we want to sleep.
> > It uses the number of instructions executed by the virtual cpu and
> > also the icount_time_shift.
> >
> > The fifth patch prints to the console whenever the guest clock runs
> > behind the host
> > clock. The fastest printing speed is every 2 seconds, and we only
> > print if the align option
> > is enabled. We also have a limit to 100 printed messages.
> >
> > The sixth patch adds information about the difference between the
> > host and guest clocks
> > (taking into account the offset) in the 'info jit' command. We also
> > print the maximum
> > delay and advance of the guest clock compared to the host clock.
> 
> I think the changes I made are too big to get this in 2.1, but we can
> certainly get this very early in 2.2 instead.  I'll shortly push my
> changes to an "icount" branch at github.com/bonzini/qemu.git.  Can
> you
> look at it and rebase patches 5 and 6 on top (plus the small change I
> asked for in my review of patch 2)?
> 
> Thanks,
> 
> Paolo
> 
> 

Sure, I'll prepare a V4 with all the changes, rebased on your "icount" branch.


Sebastian

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

[Sebastian Tanase]

----- Mail original -----
> De: "Frederic Konrad" <fred.konrad@greensocs.com>
> À: "Sebastian Tanase" <sebastian.tanase@openwide.fr>, qemu-devel@nongnu.org
> Cc: kwolf@redhat.com, "peter maydell" <peter.maydell@linaro.org>, alex@alex.org.uk, wenchaoqemu@gmail.com,
> quintela@redhat.com, mjt@tls.msk.ru, mst@redhat.com, stefanha@redhat.com, armbru@redhat.com, lcapitulino@redhat.com,
> michael@walle.cc, aliguori@amazon.com, crobinso@redhat.com, pbonzini@redhat.com, afaerber@suse.de, rth@twiddle.net
> 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 <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
> 

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)
> 
> 

Re: [Qemu-devel] [RFC PATCH V3 0/6] icount: Implement delay algorithm between guest and host clocks

[Sebastian Tanase]

----- Mail original -----
> De: "Paolo Bonzini" <pbonzini@redhat.com>
> À: "Sebastian Tanase" <sebastian.tanase@openwide.fr>, qemu-devel@nongnu.org
> Cc: aliguori@amazon.com, afaerber@suse.de, rth@twiddle.net, "peter maydell" <peter.maydell@linaro.org>,
> michael@walle.cc, alex@alex.org.uk, stefanha@redhat.com, lcapitulino@redhat.com, crobinso@redhat.com,
> armbru@redhat.com, wenchaoqemu@gmail.com, quintela@redhat.com, kwolf@redhat.com, mjt@tls.msk.ru, mst@redhat.com
> Envoyé: Lundi 30 Juin 2014 19:16:48
> Objet: Re: [RFC PATCH V3 0/6] icount: Implement delay algorithm between guest and host clocks
> 
> Il 30/06/2014 15:59, Sebastian Tanase ha scritto:
> > The icount option already implemented in QEMU allows the guest to
> > run at a theoretical
> > frequency of 1/(2^N) GHz (N is the icount parameter). The goal of
> > this patch is to have a
> > real guest frequency close to the one imposed by using the icount
> > option.
> >
> > The main idea behind the algorithm is that we compare the virtual
> > monotonic clock and the
> > host monotonic clock. For big icounts (on our test machine, an i5
> > CPU @ 3.10GHz, icounts
> > starting at 6) the guest clock will be ahead of the host clock. In
> > this case, we try to
> > sleep QEMU for the difference between the 2 clocks. Therefore, the
> > guest would have
> > executed for a period almost equally to the one imposed by icount.
> > We should point out
> > that the algorithm works only for those icounts that allow the
> > guest clock to be in front
> > of the host clock.
> >
> > The first patch adds support for QemuOpts for the 'icount'
> > parameter. It also adds a
> > suboption called 'shift' that will hold the value for 'icount'.
> > Therefore we now have
> > -icount shift=N|auto or -icount N|auto.
> >
> > The second patch adds the 'align' suboption for icount.
> >
> > The third patch exports 'icount_time_shift' so that it can be used
> > in places other than
> > cpus.c; we need it in cpu-exec.c for calculating for how long we
> > want QEMU to sleep.
> >
> > The forth patch implements the algorithm used for calculating the
> > delay we want to sleep.
> > It uses the number of instructions executed by the virtual cpu and
> > also the icount_time_shift.
> >
> > The fifth patch prints to the console whenever the guest clock runs
> > behind the host
> > clock. The fastest printing speed is every 2 seconds, and we only
> > print if the align option
> > is enabled. We also have a limit to 100 printed messages.
> >
> > The sixth patch adds information about the difference between the
> > host and guest clocks
> > (taking into account the offset) in the 'info jit' command. We also
> > print the maximum
> > delay and advance of the guest clock compared to the host clock.
> 
> I think the changes I made are too big to get this in 2.1, but we can
> certainly get this very early in 2.2 instead.  I'll shortly push my
> changes to an "icount" branch at github.com/bonzini/qemu.git.  Can
> you
> look at it and rebase patches 5 and 6 on top (plus the small change I
> asked for in my review of patch 2)?
> 
> Thanks,
> 
> Paolo
> 
> 

I've also made some changes to patch 4 (added Sleep() function call when compiling
for Windows and other small modifications); is it ok if I merge them with the existing
patch 4 on your "icount" branch and then rebase the other patches?

Sebastian

Re: [Qemu-devel] [RFC PATCH V3 0/6] icount: Implement delay algorithm between guest and host clocks

[Paolo Bonzini]

Il 04/07/2014 17:36, Sebastian Tanase ha scritto:
> I've also made some changes to patch 4 (added Sleep() function call when compiling
> for Windows and other small modifications); is it ok if I merge them with the existing
> patch 4 on your "icount" branch and then rebase the other patches?

Sure.

Paolo

Re: [Qemu-devel] [RFC PATCH V3 0/6] icount: Implement delay algorithm between guest and host clocks

[Michael Tokarev]

Can we pretty please take me off the list of recepients. I have nothing to do with this, I receive all list emails already, and where I am now it is quite dificult to sort mail, and it costs me quite some time to do so too.

When I'm back I'll concider dropping mails sent to list and Cc'd to me. Not good, because sometimes it is really useful to get attention to something important or urgent, but current practice to Cc everyone who touched the code is inacceptable and is worth filtering. IMHO anyway.

Thanks.
/mjt from android phone