[Qemu-devel] [RFC PATCH V2 1/5] icount: Add 'align' and 'icount' options

[Sebastian Tanase]

The align option is used for activating the align algorithm
in order to synchronise the host clock and the guest clock.
Therefore we slightly modified the existing icount option.
Thus, the new way to specify an icount is
'-icount [icount=][N|auto][,align]'

Signed-off-by: Sebastian Tanase <address@hidden>
Tested-by: Camille Bégué <address@hidden>
---
 cpus.c                    | 19 ++++++++++++++++++-
 include/qapi/qmp/qerror.h |  9 +++++++++
 include/qemu-common.h     |  4 +++-
 qemu-options.hx           | 19 ++++++++++++++++---
 qtest.c                   | 19 +++++++++++++++++--
 vl.c                      | 43 ++++++++++++++++++++++++++++++++++++-------
 6 files changed, 99 insertions(+), 14 deletions(-)

diff --git a/cpus.c b/cpus.c
index dd7ac13..b78a418 100644
--- a/cpus.c
+++ b/cpus.c
@@ -438,11 +438,24 @@ 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, "icount");
+    icount_align_option = qemu_opt_get_bool(opts, "align", false);
+    /* When using -icount, it is mandatory to specify an icount value:
+       -icount icount=N|auto */
     if (!option) {
+        error_set(errp, QERR_ICOUNT_MISSING);
+        return;
+    }
+    /* When using -icount icount, the icount option will be
+       misinterpreted as a boolean */
+    if (strcmp(option, "on") == 0 || strcmp(option, "off") == 0) {
+        error_set(errp, QERR_INVALID_ICOUNT);
         return;
     }
 
@@ -452,6 +465,10 @@ void configure_icount(const char *option)
         icount_time_shift = strtol(option, NULL, 0);
         use_icount = 1;
         return;
+    } else if (icount_align_option) {
+        /* Do not allow icount auto and align on */
+        error_set(errp, QERR_INVALID_ALIGN, option);
+        return;
     }
 
     use_icount = 2;
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 902d1a7..ac307d0 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -91,6 +91,15 @@ void qerror_report_err(Error *err);
 #define QERR_FEATURE_DISABLED \
     ERROR_CLASS_GENERIC_ERROR, "The feature '%s' is not enabled"
 
+#define QERR_ICOUNT_MISSING \
+    ERROR_CLASS_GENERIC_ERROR, "Icount option missing"
+
+#define QERR_INVALID_ALIGN \
+    ERROR_CLASS_GENERIC_ERROR, "Icount %s and align on are incompatible"
+
+#define QERR_INVALID_ICOUNT \
+    ERROR_CLASS_GENERIC_ERROR, "Icount must be a number or auto"
+
 #define QERR_INVALID_BLOCK_FORMAT \
     ERROR_CLASS_GENERIC_ERROR, "Invalid block format '%s'"
 
diff --git a/include/qemu-common.h b/include/qemu-common.h
index 66ceceb..288b460 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,8 +106,9 @@ 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;
+extern int icount_align_option;
 
 #include "qemu/osdep.h"
 #include "qemu/bswap.h"
diff --git a/qemu-options.hx b/qemu-options.hx
index d0714c4..5e7c956 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2898,11 +2898,11 @@ re-inject them.
 ETEXI
 
 DEF("icount", HAS_ARG, QEMU_OPTION_icount, \
-    "-icount [N|auto]\n" \
+    "-icount [icount=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
address@hidden -icount address@hidden|auto]
address@hidden -icount address@hidden|auto][,align=on|off]
 @findex -icount
 Enable virtual instruction counter.  The virtual cpu will execute one
 instruction every address@hidden ns of virtual time.  If @code{auto} is 
specified
@@ -2913,6 +2913,19 @@ 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.
+
address@hidden 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
+icount. Whenever the guest clock is behind the host clock and if
address@hidden is specified then we print a messsage to the user
+to inform about the delay.
+You must provide a value for @option{icount} other than @code{auto}
+for the align option to be taken into account.
+Currently this option does not work with @code{auto}.
+Note: The sync algorithm will work on those icounts on which
+the guest clock runs ahead of the host clock. Typically this happens
+when the icount value is high (how high depends on the host machine).
 ETEXI
 
 DEF("watchdog", HAS_ARG, QEMU_OPTION_watchdog, \
diff --git a/qtest.c b/qtest.c
index 04a6dc1..9b6c29b 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,22 @@ 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, 0);
+    Error *errp = NULL;
+    configure_icount(opts, &errp);
+    qemu_opts_del(opts);
+    if (errp != NULL) {
+        error_report("%s", error_get_pretty(errp));
+        error_free(errp);
+        exit(1);
+    }
+}
 
+int qtest_init_accel(MachineClass *mc)
+{
+    configure_qtest_icount("icount=0");
     return 0;
 }
 
diff --git a/vl.c b/vl.c
index ac0e3d7..1bd467a 100644
--- a/vl.c
+++ b/vl.c
@@ -182,6 +182,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 {
@@ -524,6 +525,21 @@ static QemuOptsList qemu_mem_opts = {
     },
 };
 
+static QemuOptsList qemu_icount_opts = {
+    .name = "icount",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_icount_opts.head),
+    .desc = {
+        {
+            .name = "icount",
+            .type = QEMU_OPT_STRING,
+        }, {
+            .name = "align",
+            .type = QEMU_OPT_BOOL,
+        },
+        { /* end of list */ }
+    },
+};
+
 /**
  * Get machine options
  *
@@ -2957,13 +2973,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;
@@ -2991,6 +3006,7 @@ int main(int argc, char **argv, char **envp)
     const char *trace_file = NULL;
     const ram_addr_t default_ram_size = (ram_addr_t)DEFAULT_RAM_SIZE *
                                         1024 * 1024;
+    Error *icount_err = NULL;
 
     atexit(qemu_run_exit_notifiers);
     error_set_progname(argv[0]);
@@ -3024,6 +3040,7 @@ int main(int argc, char **argv, char **envp)
     qemu_add_opts(&qemu_realtime_opts);
     qemu_add_opts(&qemu_msg_opts);
     qemu_add_opts(&qemu_name_opts);
+    qemu_add_opts(&qemu_icount_opts);
 
     runstate_init();
 
@@ -3833,7 +3850,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, 0);
+                if (!icount_opts) {
+                    exit(1);
+                }
                 break;
             case QEMU_OPTION_incoming:
                 incoming = optarg;
@@ -4301,11 +4322,19 @@ 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, &icount_err);
+        qemu_opts_del(icount_opts);
+        if (icount_err != NULL) {
+            error_report("%s", error_get_pretty(icount_err));
+            error_free(icount_err);
+            exit(1);
+        }
     }
-    configure_icount(icount_option);
 
     /* clean up network at qemu process termination */
     atexit(&net_cleanup);
-- 
2.0.0.rc2