All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 00/11] Final icount and misc MTTCG fixes for 2.9
@ 2017-04-10 12:55 Alex Bennée
  2017-04-10 12:55 ` [Qemu-devel] [PULL 01/11] scripts/qemugdb/mtree.py: fix up mtree dump Alex Bennée
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: Alex Bennée @ 2017-04-10 12:55 UTC (permalink / raw)
  To: peter.maydell; +Cc: qemu-devel, Alex Bennée

The following changes since commit 5fe2339e6b09da7d6f48b9bef0f1a7360392b489:

  Merge remote-tracking branch 'remotes/awilliam/tags/vfio-updates-20170406.0' into staging (2017-04-07 10:29:56 +0100)

are available in the git repository at:

  https://github.com/stsquad/qemu.git tags/pull-mttcg-fixups-for-rc2-100417-1

for you to fetch changes up to 982263ce714ffcc4c7c41a7b255bd29e093912fe:

  replay: assert time only goes forward (2017-04-10 10:23:38 +0100)

----------------------------------------------------------------
Final icount and misc MTTCG fixes for 2.9

Minor differences from:
  Message-Id: <20170405132503.32125-1-alex.bennee@linaro.org>

  - dropped new feature patches
  - last minute typo fix from Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>

----------------------------------------------------------------
Alex Bennée (10):
      scripts/qemugdb/mtree.py: fix up mtree dump
      target/i386/misc_helper: wrap BQL around another IRQ generator
      cpus: remove icount handling from qemu_tcg_cpu_thread_fn
      cpus: check cpu->running in cpu_get_icount_raw()
      cpus: move icount preparation out of tcg_exec_cpu
      cpus: don't credit executed instructions before they have run
      cpus: introduce cpu_update_icount helper
      cpu-exec: update icount after each TB_EXIT
      cpus: call cpu_update_icount on read
      replay: assert time only goes forward

Nikunj A Dadhania (1):
      cpus: fix wrong define name

 cpu-exec.c                |  14 +++---
 cpus.c                    | 111 +++++++++++++++++++++++++++++++++-------------
 include/qemu/timer.h      |   1 +
 include/qom/cpu.h         |   1 +
 replay/replay-internal.c  |   4 ++
 replay/replay.c           |   4 ++
 scripts/qemugdb/mtree.py  |  12 ++++-
 target/i386/misc_helper.c |   3 ++
 8 files changed, 110 insertions(+), 40 deletions(-)

-- 
2.11.0

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PULL 01/11] scripts/qemugdb/mtree.py: fix up mtree dump
  2017-04-10 12:55 [Qemu-devel] [PULL 00/11] Final icount and misc MTTCG fixes for 2.9 Alex Bennée
@ 2017-04-10 12:55 ` Alex Bennée
  2017-04-10 12:55 ` [Qemu-devel] [PULL 02/11] cpus: fix wrong define name Alex Bennée
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Alex Bennée @ 2017-04-10 12:55 UTC (permalink / raw)
  To: peter.maydell; +Cc: qemu-devel, Alex Bennée

Since QEMU has been able to build with native Int128 support this was
broken as it attempts to fish values out of the non-existent
structure. Also the alias print was trying to make a %x out of
gdb.ValueType directly which didn't seem to work.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

diff --git a/scripts/qemugdb/mtree.py b/scripts/qemugdb/mtree.py
index cc8131c2e7..e6791b7885 100644
--- a/scripts/qemugdb/mtree.py
+++ b/scripts/qemugdb/mtree.py
@@ -21,7 +21,15 @@ def isnull(ptr):
     return ptr == gdb.Value(0).cast(ptr.type)
 
 def int128(p):
-    return int(p['lo']) + (int(p['hi']) << 64)
+    '''Read an Int128 type to a python integer.
+
+    QEMU can be built with native Int128 support so we need to detect
+    if the value is a structure or the native type.
+    '''
+    if p.type.code == gdb.TYPE_CODE_STRUCT:
+        return int(p['lo']) + (int(p['hi']) << 64)
+    else:
+        return int(("%s" % p), 16)
 
 class MtreeCommand(gdb.Command):
     '''Display the memory tree hierarchy'''
@@ -69,7 +77,7 @@ class MtreeCommand(gdb.Command):
             gdb.write('%s    alias: %s@%016x (@ %s)\n' %
                       ('  ' * level,
                        alias['name'].string(),
-                       ptr['alias_offset'],
+                       int(ptr['alias_offset']),
                        alias,
                        ),
                       gdb.STDOUT)
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PULL 02/11] cpus: fix wrong define name
  2017-04-10 12:55 [Qemu-devel] [PULL 00/11] Final icount and misc MTTCG fixes for 2.9 Alex Bennée
  2017-04-10 12:55 ` [Qemu-devel] [PULL 01/11] scripts/qemugdb/mtree.py: fix up mtree dump Alex Bennée
@ 2017-04-10 12:55 ` Alex Bennée
  2017-04-10 12:55 ` [Qemu-devel] [PULL 03/11] target/i386/misc_helper: wrap BQL around another IRQ generator Alex Bennée
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Alex Bennée @ 2017-04-10 12:55 UTC (permalink / raw)
  To: peter.maydell
  Cc: qemu-devel, Nikunj A Dadhania, Alex Bennée, Paolo Bonzini,
	Peter Crosthwaite, Richard Henderson

From: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>

While the configure script generates TARGET_SUPPORTS_MTTCG define, one
of the define is cpus.c is checking wrong name: TARGET_SUPPORT_MTTCG

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

diff --git a/cpus.c b/cpus.c
index 68fdbc40b9..58d90aa2b9 100644
--- a/cpus.c
+++ b/cpus.c
@@ -202,7 +202,7 @@ void qemu_tcg_configure(QemuOpts *opts, Error **errp)
             } else if (use_icount) {
                 error_setg(errp, "No MTTCG when icount is enabled");
             } else {
-#ifndef TARGET_SUPPORT_MTTCG
+#ifndef TARGET_SUPPORTS_MTTCG
                 error_report("Guest not yet converted to MTTCG - "
                              "you may get unexpected results");
 #endif
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PULL 03/11] target/i386/misc_helper: wrap BQL around another IRQ generator
  2017-04-10 12:55 [Qemu-devel] [PULL 00/11] Final icount and misc MTTCG fixes for 2.9 Alex Bennée
  2017-04-10 12:55 ` [Qemu-devel] [PULL 01/11] scripts/qemugdb/mtree.py: fix up mtree dump Alex Bennée
  2017-04-10 12:55 ` [Qemu-devel] [PULL 02/11] cpus: fix wrong define name Alex Bennée
@ 2017-04-10 12:55 ` Alex Bennée
  2017-04-10 12:55 ` [Qemu-devel] [PULL 04/11] cpus: remove icount handling from qemu_tcg_cpu_thread_fn Alex Bennée
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Alex Bennée @ 2017-04-10 12:55 UTC (permalink / raw)
  To: peter.maydell
  Cc: qemu-devel, Alex Bennée, Paolo Bonzini, Richard Henderson,
	Eduardo Habkost

Anything that calls into HW emulation must be protected by the BQL.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <rth@twiddle.net>
Acked-by: Eduardo Habkost <ehabkost@redhat.com>

diff --git a/target/i386/misc_helper.c b/target/i386/misc_helper.c
index ca2ea09f54..628f64aad5 100644
--- a/target/i386/misc_helper.c
+++ b/target/i386/misc_helper.c
@@ -18,6 +18,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/main-loop.h"
 #include "cpu.h"
 #include "exec/helper-proto.h"
 #include "exec/exec-all.h"
@@ -156,7 +157,9 @@ void helper_write_crN(CPUX86State *env, int reg, target_ulong t0)
         break;
     case 8:
         if (!(env->hflags2 & HF2_VINTR_MASK)) {
+            qemu_mutex_lock_iothread();
             cpu_set_apic_tpr(x86_env_get_cpu(env)->apic_state, t0);
+            qemu_mutex_unlock_iothread();
         }
         env->v_tpr = t0 & 0x0f;
         break;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PULL 04/11] cpus: remove icount handling from qemu_tcg_cpu_thread_fn
  2017-04-10 12:55 [Qemu-devel] [PULL 00/11] Final icount and misc MTTCG fixes for 2.9 Alex Bennée
                   ` (2 preceding siblings ...)
  2017-04-10 12:55 ` [Qemu-devel] [PULL 03/11] target/i386/misc_helper: wrap BQL around another IRQ generator Alex Bennée
@ 2017-04-10 12:55 ` Alex Bennée
  2017-04-10 12:55 ` [Qemu-devel] [PULL 05/11] cpus: check cpu->running in cpu_get_icount_raw() Alex Bennée
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Alex Bennée @ 2017-04-10 12:55 UTC (permalink / raw)
  To: peter.maydell
  Cc: qemu-devel, Alex Bennée, Paolo Bonzini, Peter Crosthwaite,
	Richard Henderson

We should never be running in multi-threaded mode with icount enabled.
There is no point calling handle_icount_deadline here so remove it and
assert !use_icount.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <rth@twiddle.net>

diff --git a/cpus.c b/cpus.c
index 58d90aa2b9..fc0ddc8793 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1392,6 +1392,8 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
 {
     CPUState *cpu = arg;
 
+    g_assert(!use_icount);
+
     rcu_register_thread();
 
     qemu_mutex_lock_iothread();
@@ -1434,8 +1436,6 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
             }
         }
 
-        handle_icount_deadline();
-
         atomic_mb_set(&cpu->exit_request, 0);
         qemu_tcg_wait_io_event(cpu);
     }
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PULL 05/11] cpus: check cpu->running in cpu_get_icount_raw()
  2017-04-10 12:55 [Qemu-devel] [PULL 00/11] Final icount and misc MTTCG fixes for 2.9 Alex Bennée
                   ` (3 preceding siblings ...)
  2017-04-10 12:55 ` [Qemu-devel] [PULL 04/11] cpus: remove icount handling from qemu_tcg_cpu_thread_fn Alex Bennée
@ 2017-04-10 12:55 ` Alex Bennée
  2017-04-10 12:55 ` [Qemu-devel] [PULL 06/11] cpus: move icount preparation out of tcg_exec_cpu Alex Bennée
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Alex Bennée @ 2017-04-10 12:55 UTC (permalink / raw)
  To: peter.maydell
  Cc: qemu-devel, Alex Bennée, Paolo Bonzini, Peter Crosthwaite,
	Richard Henderson

The lifetime of current_cpu is now the lifetime of the vCPU thread.
However get_icount_raw() can apply a fudge factor if called while code
is running to take into account the current executed instruction
count.

To ensure this is always the case we also check cpu->running.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Richard Henderson <rth@twiddle.net>

diff --git a/cpus.c b/cpus.c
index fc0ddc8793..7ec6473c02 100644
--- a/cpus.c
+++ b/cpus.c
@@ -229,7 +229,7 @@ int64_t cpu_get_icount_raw(void)
     CPUState *cpu = current_cpu;
 
     icount = timers_state.qemu_icount;
-    if (cpu) {
+    if (cpu && cpu->running) {
         if (!cpu->can_do_io) {
             fprintf(stderr, "Bad icount read\n");
             exit(1);
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PULL 06/11] cpus: move icount preparation out of tcg_exec_cpu
  2017-04-10 12:55 [Qemu-devel] [PULL 00/11] Final icount and misc MTTCG fixes for 2.9 Alex Bennée
                   ` (4 preceding siblings ...)
  2017-04-10 12:55 ` [Qemu-devel] [PULL 05/11] cpus: check cpu->running in cpu_get_icount_raw() Alex Bennée
@ 2017-04-10 12:55 ` Alex Bennée
  2017-04-10 12:55 ` [Qemu-devel] [PULL 07/11] cpus: don't credit executed instructions before they have run Alex Bennée
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Alex Bennée @ 2017-04-10 12:55 UTC (permalink / raw)
  To: peter.maydell
  Cc: qemu-devel, Alex Bennée, Paolo Bonzini, Peter Crosthwaite,
	Richard Henderson

As icount is only supported for single-threaded execution due to the
requirement for determinism let's remove it from the common
tcg_exec_cpu path.

Also remove the additional fiddling which shouldn't be required as the
icount counters should all be rectified as you enter the loop.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

diff --git a/cpus.c b/cpus.c
index 7ec6473c02..6034b104c3 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1179,47 +1179,64 @@ static void handle_icount_deadline(void)
     }
 }
 
-static int tcg_cpu_exec(CPUState *cpu)
+static void prepare_icount_for_run(CPUState *cpu)
 {
-    int ret;
-#ifdef CONFIG_PROFILER
-    int64_t ti;
-#endif
-
-#ifdef CONFIG_PROFILER
-    ti = profile_getclock();
-#endif
     if (use_icount) {
         int64_t count;
         int decr;
-        timers_state.qemu_icount -= (cpu->icount_decr.u16.low
-                                    + cpu->icount_extra);
-        cpu->icount_decr.u16.low = 0;
-        cpu->icount_extra = 0;
+
+        /* These should always be cleared by process_icount_data after
+         * each vCPU execution. However u16.high can be raised
+         * asynchronously by cpu_exit/cpu_interrupt/tcg_handle_interrupt
+         */
+        g_assert(cpu->icount_decr.u16.low == 0);
+        g_assert(cpu->icount_extra == 0);
+
+
         count = tcg_get_icount_limit();
+
         timers_state.qemu_icount += count;
         decr = (count > 0xffff) ? 0xffff : count;
         count -= decr;
         cpu->icount_decr.u16.low = decr;
         cpu->icount_extra = count;
     }
-    qemu_mutex_unlock_iothread();
-    cpu_exec_start(cpu);
-    ret = cpu_exec(cpu);
-    cpu_exec_end(cpu);
-    qemu_mutex_lock_iothread();
-#ifdef CONFIG_PROFILER
-    tcg_time += profile_getclock() - ti;
-#endif
+}
+
+static void process_icount_data(CPUState *cpu)
+{
     if (use_icount) {
         /* Fold pending instructions back into the
            instruction counter, and clear the interrupt flag.  */
         timers_state.qemu_icount -= (cpu->icount_decr.u16.low
                         + cpu->icount_extra);
-        cpu->icount_decr.u32 = 0;
+
+        /* Reset the counters */
+        cpu->icount_decr.u16.low = 0;
         cpu->icount_extra = 0;
         replay_account_executed_instructions();
     }
+}
+
+
+static int tcg_cpu_exec(CPUState *cpu)
+{
+    int ret;
+#ifdef CONFIG_PROFILER
+    int64_t ti;
+#endif
+
+#ifdef CONFIG_PROFILER
+    ti = profile_getclock();
+#endif
+    qemu_mutex_unlock_iothread();
+    cpu_exec_start(cpu);
+    ret = cpu_exec(cpu);
+    cpu_exec_end(cpu);
+    qemu_mutex_lock_iothread();
+#ifdef CONFIG_PROFILER
+    tcg_time += profile_getclock() - ti;
+#endif
     return ret;
 }
 
@@ -1306,7 +1323,13 @@ static void *qemu_tcg_rr_cpu_thread_fn(void *arg)
 
             if (cpu_can_run(cpu)) {
                 int r;
+
+                prepare_icount_for_run(cpu);
+
                 r = tcg_cpu_exec(cpu);
+
+                process_icount_data(cpu);
+
                 if (r == EXCP_DEBUG) {
                     cpu_handle_guest_debug(cpu);
                     break;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PULL 07/11] cpus: don't credit executed instructions before they have run
  2017-04-10 12:55 [Qemu-devel] [PULL 00/11] Final icount and misc MTTCG fixes for 2.9 Alex Bennée
                   ` (5 preceding siblings ...)
  2017-04-10 12:55 ` [Qemu-devel] [PULL 06/11] cpus: move icount preparation out of tcg_exec_cpu Alex Bennée
@ 2017-04-10 12:55 ` Alex Bennée
  2017-04-10 12:55 ` [Qemu-devel] [PULL 08/11] cpus: introduce cpu_update_icount helper Alex Bennée
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Alex Bennée @ 2017-04-10 12:55 UTC (permalink / raw)
  To: peter.maydell
  Cc: qemu-devel, Alex Bennée, Paolo Bonzini, Peter Crosthwaite,
	Richard Henderson

Outside of the vCPU thread icount time will only be tracked against
timers_state.qemu_icount. We no longer credit cycles until they have
completed the run. Inside the vCPU thread we adjust for passage of
time by looking at how many have run so far. This is only valid inside
the vCPU thread while it is running.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

diff --git a/cpus.c b/cpus.c
index 6034b104c3..0ecb0b87f0 100644
--- a/cpus.c
+++ b/cpus.c
@@ -223,6 +223,15 @@ void qemu_tcg_configure(QemuOpts *opts, Error **errp)
     }
 }
 
+/* The current number of executed instructions is based on what we
+ * originally budgeted minus the current state of the decrementing
+ * icount counters in extra/u16.low.
+ */
+static int64_t cpu_get_icount_executed(CPUState *cpu)
+{
+    return cpu->icount_budget - (cpu->icount_decr.u16.low + cpu->icount_extra);
+}
+
 int64_t cpu_get_icount_raw(void)
 {
     int64_t icount;
@@ -234,7 +243,8 @@ int64_t cpu_get_icount_raw(void)
             fprintf(stderr, "Bad icount read\n");
             exit(1);
         }
-        icount -= (cpu->icount_decr.u16.low + cpu->icount_extra);
+        /* Take into account what has run */
+        icount += cpu_get_icount_executed(cpu);
     }
     return icount;
 }
@@ -1195,7 +1205,10 @@ static void prepare_icount_for_run(CPUState *cpu)
 
         count = tcg_get_icount_limit();
 
-        timers_state.qemu_icount += count;
+        /* To calculate what we have executed so far we need to know
+         * what we originally budgeted to run this cycle */
+        cpu->icount_budget = count;
+
         decr = (count > 0xffff) ? 0xffff : count;
         count -= decr;
         cpu->icount_decr.u16.low = decr;
@@ -1206,14 +1219,14 @@ static void prepare_icount_for_run(CPUState *cpu)
 static void process_icount_data(CPUState *cpu)
 {
     if (use_icount) {
-        /* Fold pending instructions back into the
-           instruction counter, and clear the interrupt flag.  */
-        timers_state.qemu_icount -= (cpu->icount_decr.u16.low
-                        + cpu->icount_extra);
+        /* Account for executed instructions */
+        timers_state.qemu_icount += cpu_get_icount_executed(cpu);
 
         /* Reset the counters */
         cpu->icount_decr.u16.low = 0;
         cpu->icount_extra = 0;
+        cpu->icount_budget = 0;
+
         replay_account_executed_instructions();
     }
 }
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index c3292efe1c..5d10359c8f 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -332,6 +332,7 @@ struct CPUState {
     /* updates protected by BQL */
     uint32_t interrupt_request;
     int singlestep_enabled;
+    int64_t icount_budget;
     int64_t icount_extra;
     sigjmp_buf jmp_env;
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PULL 08/11] cpus: introduce cpu_update_icount helper
  2017-04-10 12:55 [Qemu-devel] [PULL 00/11] Final icount and misc MTTCG fixes for 2.9 Alex Bennée
                   ` (6 preceding siblings ...)
  2017-04-10 12:55 ` [Qemu-devel] [PULL 07/11] cpus: don't credit executed instructions before they have run Alex Bennée
@ 2017-04-10 12:55 ` Alex Bennée
  2017-04-10 12:55 ` [Qemu-devel] [PULL 09/11] cpu-exec: update icount after each TB_EXIT Alex Bennée
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Alex Bennée @ 2017-04-10 12:55 UTC (permalink / raw)
  To: peter.maydell
  Cc: qemu-devel, Alex Bennée, Paolo Bonzini, Peter Crosthwaite,
	Richard Henderson

By holding off updates to timer_state.qemu_icount we can run into
trouble when the non-vCPU thread needs to know the time. This helper
ensures we atomically update timers_state.qemu_icount based on what
has been currently executed.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

diff --git a/cpus.c b/cpus.c
index 0ecb0b87f0..a5125d7167 100644
--- a/cpus.c
+++ b/cpus.c
@@ -232,12 +232,31 @@ static int64_t cpu_get_icount_executed(CPUState *cpu)
     return cpu->icount_budget - (cpu->icount_decr.u16.low + cpu->icount_extra);
 }
 
+/*
+ * Update the global shared timer_state.qemu_icount to take into
+ * account executed instructions. This is done by the TCG vCPU
+ * thread so the main-loop can see time has moved forward.
+ */
+void cpu_update_icount(CPUState *cpu)
+{
+    int64_t executed = cpu_get_icount_executed(cpu);
+    cpu->icount_budget -= executed;
+
+#ifdef CONFIG_ATOMIC64
+    atomic_set__nocheck(&timers_state.qemu_icount,
+                        atomic_read__nocheck(&timers_state.qemu_icount) +
+                        executed);
+#else /* FIXME: we need 64bit atomics to do this safely */
+    timers_state.qemu_icount += executed;
+#endif
+}
+
 int64_t cpu_get_icount_raw(void)
 {
     int64_t icount;
     CPUState *cpu = current_cpu;
 
-    icount = timers_state.qemu_icount;
+    icount = atomic_read(&timers_state.qemu_icount);
     if (cpu && cpu->running) {
         if (!cpu->can_do_io) {
             fprintf(stderr, "Bad icount read\n");
@@ -1220,7 +1239,7 @@ static void process_icount_data(CPUState *cpu)
 {
     if (use_icount) {
         /* Account for executed instructions */
-        timers_state.qemu_icount += cpu_get_icount_executed(cpu);
+        cpu_update_icount(cpu);
 
         /* Reset the counters */
         cpu->icount_decr.u16.low = 0;
diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index e1742f2f3d..8a1eb74839 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -869,6 +869,7 @@ int64_t cpu_get_icount_raw(void);
 int64_t cpu_get_icount(void);
 int64_t cpu_get_clock(void);
 int64_t cpu_icount_to_ns(int64_t icount);
+void    cpu_update_icount(CPUState *cpu);
 
 /*******************************************/
 /* host CPU ticks (if available) */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PULL 09/11] cpu-exec: update icount after each TB_EXIT
  2017-04-10 12:55 [Qemu-devel] [PULL 00/11] Final icount and misc MTTCG fixes for 2.9 Alex Bennée
                   ` (7 preceding siblings ...)
  2017-04-10 12:55 ` [Qemu-devel] [PULL 08/11] cpus: introduce cpu_update_icount helper Alex Bennée
@ 2017-04-10 12:55 ` Alex Bennée
  2017-04-10 12:55 ` [Qemu-devel] [PULL 10/11] cpus: call cpu_update_icount on read Alex Bennée
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Alex Bennée @ 2017-04-10 12:55 UTC (permalink / raw)
  To: peter.maydell
  Cc: qemu-devel, Alex Bennée, Paolo Bonzini, Peter Crosthwaite,
	Richard Henderson

There is no particular reason we shouldn't update the global system
icount time as we exit each TranslationBlock run. This ensures the
main-loop doesn't have to wait until we exit to the outer loop for
executed instructions to be credited to timer_state.

The prepare_icount_for_run function is slightly tweaked to match the
logic we run in cpu_loop_exec_tb.

Based on Paolo's original suggestion.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

diff --git a/cpu-exec.c b/cpu-exec.c
index 748cb66bca..63a56d0407 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -600,13 +600,13 @@ static inline void cpu_loop_exec_tb(CPUState *cpu, TranslationBlock *tb,
     /* Instruction counter expired.  */
     assert(use_icount);
 #ifndef CONFIG_USER_ONLY
-    if (cpu->icount_extra) {
-        /* Refill decrementer and continue execution.  */
-        cpu->icount_extra += insns_left;
-        insns_left = MIN(0xffff, cpu->icount_extra);
-        cpu->icount_extra -= insns_left;
-        cpu->icount_decr.u16.low = insns_left;
-    } else {
+    /* Ensure global icount has gone forward */
+    cpu_update_icount(cpu);
+    /* Refill decrementer and continue execution.  */
+    insns_left = MIN(0xffff, cpu->icount_budget);
+    cpu->icount_decr.u16.low = insns_left;
+    cpu->icount_extra = cpu->icount_budget - insns_left;
+    if (!cpu->icount_extra) {
         /* Execute any remaining instructions, then let the main loop
          * handle the next event.
          */
diff --git a/cpus.c b/cpus.c
index a5125d7167..9c8bd2c991 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1211,8 +1211,7 @@ static void handle_icount_deadline(void)
 static void prepare_icount_for_run(CPUState *cpu)
 {
     if (use_icount) {
-        int64_t count;
-        int decr;
+        int insns_left;
 
         /* These should always be cleared by process_icount_data after
          * each vCPU execution. However u16.high can be raised
@@ -1221,17 +1220,10 @@ static void prepare_icount_for_run(CPUState *cpu)
         g_assert(cpu->icount_decr.u16.low == 0);
         g_assert(cpu->icount_extra == 0);
 
-
-        count = tcg_get_icount_limit();
-
-        /* To calculate what we have executed so far we need to know
-         * what we originally budgeted to run this cycle */
-        cpu->icount_budget = count;
-
-        decr = (count > 0xffff) ? 0xffff : count;
-        count -= decr;
-        cpu->icount_decr.u16.low = decr;
-        cpu->icount_extra = count;
+        cpu->icount_budget = tcg_get_icount_limit();
+        insns_left = MIN(0xffff, cpu->icount_budget);
+        cpu->icount_decr.u16.low = insns_left;
+        cpu->icount_extra = cpu->icount_budget - insns_left;
     }
 }
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PULL 10/11] cpus: call cpu_update_icount on read
  2017-04-10 12:55 [Qemu-devel] [PULL 00/11] Final icount and misc MTTCG fixes for 2.9 Alex Bennée
                   ` (8 preceding siblings ...)
  2017-04-10 12:55 ` [Qemu-devel] [PULL 09/11] cpu-exec: update icount after each TB_EXIT Alex Bennée
@ 2017-04-10 12:55 ` Alex Bennée
  2017-04-10 12:55 ` [Qemu-devel] [PULL 11/11] replay: assert time only goes forward Alex Bennée
  2017-04-10 15:08 ` [Qemu-devel] [PULL 00/11] Final icount and misc MTTCG fixes for 2.9 Peter Maydell
  11 siblings, 0 replies; 13+ messages in thread
From: Alex Bennée @ 2017-04-10 12:55 UTC (permalink / raw)
  To: peter.maydell
  Cc: qemu-devel, Alex Bennée, Paolo Bonzini, Peter Crosthwaite,
	Richard Henderson

This ensures each time the vCPU thread reads the icount we update the
master timer_state.qemu_icount field. This way as long as updates are
in BQL protected sections (which they should be) the main-loop can
never come to update the log and find time has gone backwards.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

diff --git a/cpus.c b/cpus.c
index 9c8bd2c991..740b8dc3f8 100644
--- a/cpus.c
+++ b/cpus.c
@@ -253,19 +253,21 @@ void cpu_update_icount(CPUState *cpu)
 
 int64_t cpu_get_icount_raw(void)
 {
-    int64_t icount;
     CPUState *cpu = current_cpu;
 
-    icount = atomic_read(&timers_state.qemu_icount);
     if (cpu && cpu->running) {
         if (!cpu->can_do_io) {
             fprintf(stderr, "Bad icount read\n");
             exit(1);
         }
         /* Take into account what has run */
-        icount += cpu_get_icount_executed(cpu);
+        cpu_update_icount(cpu);
     }
-    return icount;
+#ifdef CONFIG_ATOMIC64
+    return atomic_read__nocheck(&timers_state.qemu_icount);
+#else /* FIXME: we need 64bit atomics to do this safely */
+    return timers_state.qemu_icount;
+#endif
 }
 
 /* Return the virtual CPU time, based on the instruction counter.  */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PULL 11/11] replay: assert time only goes forward
  2017-04-10 12:55 [Qemu-devel] [PULL 00/11] Final icount and misc MTTCG fixes for 2.9 Alex Bennée
                   ` (9 preceding siblings ...)
  2017-04-10 12:55 ` [Qemu-devel] [PULL 10/11] cpus: call cpu_update_icount on read Alex Bennée
@ 2017-04-10 12:55 ` Alex Bennée
  2017-04-10 15:08 ` [Qemu-devel] [PULL 00/11] Final icount and misc MTTCG fixes for 2.9 Peter Maydell
  11 siblings, 0 replies; 13+ messages in thread
From: Alex Bennée @ 2017-04-10 12:55 UTC (permalink / raw)
  To: peter.maydell; +Cc: qemu-devel, Alex Bennée

If we find ourselves trying to add an event to the log where time has
gone backwards it is because a vCPU event has occurred and the
main-loop is not yet aware of time moving forward. This should not
happen and if it does its better to fail early than generate a log
that will have weird behaviour.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

diff --git a/replay/replay-internal.c b/replay/replay-internal.c
index bea7b4aa6b..fca8514012 100644
--- a/replay/replay-internal.c
+++ b/replay/replay-internal.c
@@ -195,6 +195,10 @@ void replay_save_instructions(void)
     if (replay_file && replay_mode == REPLAY_MODE_RECORD) {
         replay_mutex_lock();
         int diff = (int)(replay_get_current_step() - replay_state.current_step);
+
+        /* Time can only go forward */
+        assert(diff >= 0);
+
         if (diff > 0) {
             replay_put_event(EVENT_INSTRUCTION);
             replay_put_dword(diff);
diff --git a/replay/replay.c b/replay/replay.c
index 9e0724e756..f810628cac 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -84,6 +84,10 @@ void replay_account_executed_instructions(void)
         if (replay_state.instructions_count > 0) {
             int count = (int)(replay_get_current_step()
                               - replay_state.current_step);
+
+            /* Time can only go forward */
+            assert(count >= 0);
+
             replay_state.instructions_count -= count;
             replay_state.current_step += count;
             if (replay_state.instructions_count == 0) {
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PULL 00/11] Final icount and misc MTTCG fixes for 2.9
  2017-04-10 12:55 [Qemu-devel] [PULL 00/11] Final icount and misc MTTCG fixes for 2.9 Alex Bennée
                   ` (10 preceding siblings ...)
  2017-04-10 12:55 ` [Qemu-devel] [PULL 11/11] replay: assert time only goes forward Alex Bennée
@ 2017-04-10 15:08 ` Peter Maydell
  11 siblings, 0 replies; 13+ messages in thread
From: Peter Maydell @ 2017-04-10 15:08 UTC (permalink / raw)
  To: Alex Bennée; +Cc: QEMU Developers

On 10 April 2017 at 13:55, Alex Bennée <alex.bennee@linaro.org> wrote:
> The following changes since commit 5fe2339e6b09da7d6f48b9bef0f1a7360392b489:
>
>   Merge remote-tracking branch 'remotes/awilliam/tags/vfio-updates-20170406.0' into staging (2017-04-07 10:29:56 +0100)
>
> are available in the git repository at:
>
>   https://github.com/stsquad/qemu.git tags/pull-mttcg-fixups-for-rc2-100417-1
>
> for you to fetch changes up to 982263ce714ffcc4c7c41a7b255bd29e093912fe:
>
>   replay: assert time only goes forward (2017-04-10 10:23:38 +0100)
>
> ----------------------------------------------------------------
> Final icount and misc MTTCG fixes for 2.9
>
> Minor differences from:
>   Message-Id: <20170405132503.32125-1-alex.bennee@linaro.org>
>
>   - dropped new feature patches
>   - last minute typo fix from Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>
> ----------------------------------------------------------------

Applied, thanks.

-- PMM

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2017-04-10 15:09 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-10 12:55 [Qemu-devel] [PULL 00/11] Final icount and misc MTTCG fixes for 2.9 Alex Bennée
2017-04-10 12:55 ` [Qemu-devel] [PULL 01/11] scripts/qemugdb/mtree.py: fix up mtree dump Alex Bennée
2017-04-10 12:55 ` [Qemu-devel] [PULL 02/11] cpus: fix wrong define name Alex Bennée
2017-04-10 12:55 ` [Qemu-devel] [PULL 03/11] target/i386/misc_helper: wrap BQL around another IRQ generator Alex Bennée
2017-04-10 12:55 ` [Qemu-devel] [PULL 04/11] cpus: remove icount handling from qemu_tcg_cpu_thread_fn Alex Bennée
2017-04-10 12:55 ` [Qemu-devel] [PULL 05/11] cpus: check cpu->running in cpu_get_icount_raw() Alex Bennée
2017-04-10 12:55 ` [Qemu-devel] [PULL 06/11] cpus: move icount preparation out of tcg_exec_cpu Alex Bennée
2017-04-10 12:55 ` [Qemu-devel] [PULL 07/11] cpus: don't credit executed instructions before they have run Alex Bennée
2017-04-10 12:55 ` [Qemu-devel] [PULL 08/11] cpus: introduce cpu_update_icount helper Alex Bennée
2017-04-10 12:55 ` [Qemu-devel] [PULL 09/11] cpu-exec: update icount after each TB_EXIT Alex Bennée
2017-04-10 12:55 ` [Qemu-devel] [PULL 10/11] cpus: call cpu_update_icount on read Alex Bennée
2017-04-10 12:55 ` [Qemu-devel] [PULL 11/11] replay: assert time only goes forward Alex Bennée
2017-04-10 15:08 ` [Qemu-devel] [PULL 00/11] Final icount and misc MTTCG fixes for 2.9 Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.