[Qemu-devel] [PULL 00/11] Final icount and misc MTTCG fixes for 2.9

Alex Bennée posted 11 patches 7 years ago
Only 4 patches received!
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(-)
[Qemu-devel] [PULL 00/11] Final icount and misc MTTCG fixes for 2.9
Posted by Alex Bennée 7 years ago
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


[Qemu-devel] [PULL 01/11] scripts/qemugdb/mtree.py: fix up mtree dump
Posted by Alex Bennée 7 years ago
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


[Qemu-devel] [PULL 02/11] cpus: fix wrong define name
Posted by Alex Bennée 7 years ago
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


[Qemu-devel] [PULL 03/11] target/i386/misc_helper: wrap BQL around another IRQ generator
Posted by Alex Bennée 7 years ago
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


[Qemu-devel] [PULL 04/11] cpus: remove icount handling from qemu_tcg_cpu_thread_fn
Posted by Alex Bennée 7 years ago
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


[Qemu-devel] [PULL 05/11] cpus: check cpu->running in cpu_get_icount_raw()
Posted by Alex Bennée 7 years ago
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


[Qemu-devel] [PULL 06/11] cpus: move icount preparation out of tcg_exec_cpu
Posted by Alex Bennée 7 years ago
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


[Qemu-devel] [PULL 10/11] cpus: call cpu_update_icount on read
Posted by Alex Bennée 7 years ago
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


Re: [Qemu-devel] [PULL 00/11] Final icount and misc MTTCG fixes for 2.9
Posted by Peter Maydell 7 years ago
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