[PATCH v3 0/6] sysemu/replay: Restrict icount to TCG system emulation

[PATCH v3 0/6] sysemu/replay: Restrict icount to TCG system emulation

[Philippe Mathieu-Daudé]

Slightly simplify non-TCG and user emulation code.

This series still adds assertions in ARM INST_RETIRED
PMU events, in order to bypass a linking failure. Better
would be to restrict ARM PMU events to TCG. Left for
another series.

Since v2:
- Have icount_configure() return bool
- Addressed rth's review comments

Since v1:
- Introduce enum of icount modes
- Fix ARM INST_RETIRED event

Philippe Mathieu-Daudé (6):
  sysemu/cpu-timers: Have icount_configure() return a boolean
  system/vl: Evaluate icount after accelerator options are parsed
  sysemu/cpu-timers: Introduce ICountMode enumerator
  target/arm: Ensure icount is enabled when emulating INST_RETIRED
  util/async: Only call icount_notify_exit() if icount is enabled
  sysemu/replay: Restrict icount to system emulation

 include/sysemu/cpu-timers.h | 32 ++++++++++++++++++++++----------
 include/sysemu/replay.h     | 11 ++++++++---
 accel/tcg/icount-common.c   | 36 +++++++++++++++++++-----------------
 stubs/icount.c              | 29 ++---------------------------
 system/cpu-timers.c         |  2 +-
 system/vl.c                 | 19 ++++++++++---------
 target/arm/helper.c         |  5 ++++-
 util/async.c                | 16 +++++++++-------
 8 files changed, 75 insertions(+), 75 deletions(-)

-- 
2.41.0

[PATCH v3 1/6] sysemu/cpu-timers: Have icount_configure() return a boolean

[Philippe Mathieu-Daudé]

Following the example documented since commit e3fe3988d7 ("error:
Document Error API usage rules"), have icount_configure()
return a boolean indicating whether an error is set or not.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/sysemu/cpu-timers.h | 10 ++++++++--
 accel/tcg/icount-common.c   | 16 +++++++++-------
 stubs/icount.c              |  4 +++-
 system/vl.c                 |  3 +--
 4 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/include/sysemu/cpu-timers.h b/include/sysemu/cpu-timers.h
index 2e786fe7fb..b70dc7692d 100644
--- a/include/sysemu/cpu-timers.h
+++ b/include/sysemu/cpu-timers.h
@@ -50,8 +50,14 @@ int64_t icount_get(void);
  */
 int64_t icount_to_ns(int64_t icount);
 
-/* configure the icount options, including "shift" */
-void icount_configure(QemuOpts *opts, Error **errp);
+/**
+ * icount_configure: configure the icount options, including "shift"
+ * @opts: Options to parse
+ * @errp: pointer to a NULL-initialized error object
+ *
+ * Return: true on success, else false setting @errp with error
+ */
+bool icount_configure(QemuOpts *opts, Error **errp);
 
 /* used by tcg vcpu thread to calc icount budget */
 int64_t icount_round(int64_t count);
diff --git a/accel/tcg/icount-common.c b/accel/tcg/icount-common.c
index ec57192be8..dc69d6a4c6 100644
--- a/accel/tcg/icount-common.c
+++ b/accel/tcg/icount-common.c
@@ -419,7 +419,7 @@ void icount_account_warp_timer(void)
     icount_warp_rt();
 }
 
-void icount_configure(QemuOpts *opts, Error **errp)
+bool icount_configure(QemuOpts *opts, Error **errp)
 {
     const char *option = qemu_opt_get(opts, "shift");
     bool sleep = qemu_opt_get_bool(opts, "sleep", true);
@@ -429,27 +429,28 @@ void icount_configure(QemuOpts *opts, Error **errp)
     if (!option) {
         if (qemu_opt_get(opts, "align") != NULL) {
             error_setg(errp, "Please specify shift option when using align");
+            return false;
         }
-        return;
+        return true;
     }
 
     if (align && !sleep) {
         error_setg(errp, "align=on and sleep=off are incompatible");
-        return;
+        return false;
     }
 
     if (strcmp(option, "auto") != 0) {
         if (qemu_strtol(option, NULL, 0, &time_shift) < 0
             || time_shift < 0 || time_shift > MAX_ICOUNT_SHIFT) {
             error_setg(errp, "icount: Invalid shift value");
-            return;
+            return false;
         }
     } else if (icount_align_option) {
         error_setg(errp, "shift=auto and align=on are incompatible");
-        return;
+        return false;
     } else if (!icount_sleep) {
         error_setg(errp, "shift=auto and sleep=off are incompatible");
-        return;
+        return false;
     }
 
     icount_sleep = sleep;
@@ -463,7 +464,7 @@ void icount_configure(QemuOpts *opts, Error **errp)
     if (time_shift >= 0) {
         timers_state.icount_time_shift = time_shift;
         icount_enable_precise();
-        return;
+        return true;
     }
 
     icount_enable_adaptive();
@@ -491,6 +492,7 @@ void icount_configure(QemuOpts *opts, Error **errp)
     timer_mod(timers_state.icount_vm_timer,
                    qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
                    NANOSECONDS_PER_SECOND / 10);
+    return true;
 }
 
 void icount_notify_exit(void)
diff --git a/stubs/icount.c b/stubs/icount.c
index 6df8c2bf7d..85c381a0ea 100644
--- a/stubs/icount.c
+++ b/stubs/icount.c
@@ -10,10 +10,12 @@ void icount_update(CPUState *cpu)
 {
     abort();
 }
-void icount_configure(QemuOpts *opts, Error **errp)
+bool icount_configure(QemuOpts *opts, Error **errp)
 {
     /* signal error */
     error_setg(errp, "cannot configure icount, TCG support not available");
+
+    return false;
 }
 int64_t icount_get_raw(void)
 {
diff --git a/system/vl.c b/system/vl.c
index 2bcd9efb9a..17234012d4 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -2270,8 +2270,7 @@ static void user_register_global_props(void)
 
 static int do_configure_icount(void *opaque, QemuOpts *opts, Error **errp)
 {
-    icount_configure(opts, errp);
-    return 0;
+    return !icount_configure(opts, errp);
 }
 
 static int accelerator_set_property(void *opaque,
-- 
2.41.0

[PATCH v3 2/6] system/vl: Evaluate icount after accelerator options are parsed

[Philippe Mathieu-Daudé]

We need to parse the accelerators first, to be able
to check whether TCG is enabled or not. Then we can
parse the -icount option.

This allows removing the icount_configure() stub.

Fixes: 7f8b6126e7 ("vl: move icount configuration earlier")
Reported-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 stubs/icount.c |  8 --------
 system/vl.c    | 16 +++++++++-------
 2 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/stubs/icount.c b/stubs/icount.c
index 85c381a0ea..014ae5d8e4 100644
--- a/stubs/icount.c
+++ b/stubs/icount.c
@@ -1,5 +1,4 @@
 #include "qemu/osdep.h"
-#include "qapi/error.h"
 #include "sysemu/cpu-timers.h"
 
 /* icount - Instruction Counter API */
@@ -10,13 +9,6 @@ void icount_update(CPUState *cpu)
 {
     abort();
 }
-bool icount_configure(QemuOpts *opts, Error **errp)
-{
-    /* signal error */
-    error_setg(errp, "cannot configure icount, TCG support not available");
-
-    return false;
-}
 int64_t icount_get_raw(void)
 {
     abort();
diff --git a/system/vl.c b/system/vl.c
index 17234012d4..af809331bb 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -2270,6 +2270,13 @@ static void user_register_global_props(void)
 
 static int do_configure_icount(void *opaque, QemuOpts *opts, Error **errp)
 {
+    if (!tcg_enabled()) {
+        error_setg(errp, "cannot configure icount, TCG support not available");
+        error_append_hint(errp, "-icount is not allowed with"
+                                " hardware virtualization\n");
+        return 1;
+    }
+
     return !icount_configure(opts, errp);
 }
 
@@ -2339,9 +2346,6 @@ static void configure_accelerators(const char *progname)
 {
     bool init_failed = false;
 
-    qemu_opts_foreach(qemu_find_opts("icount"),
-                      do_configure_icount, NULL, &error_fatal);
-
     if (QTAILQ_EMPTY(&qemu_accel_opts.head)) {
         char **accel_list, **tmp;
 
@@ -2401,10 +2405,8 @@ static void configure_accelerators(const char *progname)
         error_report("falling back to %s", current_accel_name());
     }
 
-    if (icount_enabled() && !tcg_enabled()) {
-        error_report("-icount is not allowed with hardware virtualization");
-        exit(1);
-    }
+    qemu_opts_foreach(qemu_find_opts("icount"),
+                      do_configure_icount, NULL, &error_fatal);
 }
 
 static void qemu_validate_options(const QDict *machine_opts)
-- 
2.41.0

[PATCH v3 3/6] sysemu/cpu-timers: Introduce ICountMode enumerator

[Philippe Mathieu-Daudé]

Rather than having to lookup for what the 0, 1, 2, ...
icount values are, use a enum definition.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/sysemu/cpu-timers.h | 20 +++++++++++++-------
 accel/tcg/icount-common.c   | 16 +++++++---------
 stubs/icount.c              |  2 +-
 system/cpu-timers.c         |  2 +-
 target/arm/helper.c         |  3 ++-
 5 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/include/sysemu/cpu-timers.h b/include/sysemu/cpu-timers.h
index b70dc7692d..3f05f29b10 100644
--- a/include/sysemu/cpu-timers.h
+++ b/include/sysemu/cpu-timers.h
@@ -17,18 +17,24 @@ void cpu_timers_init(void);
 
 /* icount - Instruction Counter API */
 
-/*
- * icount enablement state:
+/**
+ * ICountMode: icount enablement state:
  *
- * 0 = Disabled - Do not count executed instructions.
- * 1 = Enabled - Fixed conversion of insn to ns via "shift" option
- * 2 = Enabled - Runtime adaptive algorithm to compute shift
+ * @ICOUNT_DISABLED: Disabled - Do not count executed instructions.
+ * @ICOUNT_PRECISE: Enabled - Fixed conversion of insn to ns via "shift" option
+ * @ICOUNT_ADAPTATIVE: Enabled - Runtime adaptive algorithm to compute shift
  */
+typedef enum {
+    ICOUNT_DISABLED = 0,
+    ICOUNT_PRECISE,
+    ICOUNT_ADAPTATIVE,
+} ICountMode;
+
 #ifdef CONFIG_TCG
-extern int use_icount;
+extern ICountMode use_icount;
 #define icount_enabled() (use_icount)
 #else
-#define icount_enabled() 0
+#define icount_enabled() ICOUNT_DISABLED
 #endif
 
 /*
diff --git a/accel/tcg/icount-common.c b/accel/tcg/icount-common.c
index dc69d6a4c6..f0f8fc7f1c 100644
--- a/accel/tcg/icount-common.c
+++ b/accel/tcg/icount-common.c
@@ -49,21 +49,19 @@ static bool icount_sleep = true;
 /* Arbitrarily pick 1MIPS as the minimum allowable speed.  */
 #define MAX_ICOUNT_SHIFT 10
 
-/*
- * 0 = Do not count executed instructions.
- * 1 = Fixed conversion of insn to ns via "shift" option
- * 2 = Runtime adaptive algorithm to compute shift
- */
-int use_icount;
+/* Do not count executed instructions */
+ICountMode use_icount = ICOUNT_DISABLED;
 
 static void icount_enable_precise(void)
 {
-    use_icount = 1;
+    /* Fixed conversion of insn to ns via "shift" option */
+    use_icount = ICOUNT_PRECISE;
 }
 
 static void icount_enable_adaptive(void)
 {
-    use_icount = 2;
+    /* Runtime adaptive algorithm to compute shift */
+    use_icount = ICOUNT_ADAPTATIVE;
 }
 
 /*
@@ -256,7 +254,7 @@ static void icount_warp_rt(void)
         int64_t warp_delta;
 
         warp_delta = clock - timers_state.vm_clock_warp_start;
-        if (icount_enabled() == 2) {
+        if (icount_enabled() == ICOUNT_ADAPTATIVE) {
             /*
              * In adaptive mode, do not let QEMU_CLOCK_VIRTUAL run too far
              * ahead of real time (it might already be ahead so careful not
diff --git a/stubs/icount.c b/stubs/icount.c
index 014ae5d8e4..7055c13725 100644
--- a/stubs/icount.c
+++ b/stubs/icount.c
@@ -3,7 +3,7 @@
 
 /* icount - Instruction Counter API */
 
-int use_icount;
+ICountMode use_icount = ICOUNT_DISABLED;
 
 void icount_update(CPUState *cpu)
 {
diff --git a/system/cpu-timers.c b/system/cpu-timers.c
index 7452d97b67..6befb82e48 100644
--- a/system/cpu-timers.c
+++ b/system/cpu-timers.c
@@ -154,7 +154,7 @@ static bool adjust_timers_state_needed(void *opaque)
 
 static bool icount_shift_state_needed(void *opaque)
 {
-    return icount_enabled() == 2;
+    return icount_enabled() == ICOUNT_ADAPTATIVE;
 }
 
 /*
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 2746d3fdac..adb0960bba 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -934,7 +934,8 @@ static int64_t cycles_ns_per(uint64_t cycles)
 
 static bool instructions_supported(CPUARMState *env)
 {
-    return icount_enabled() == 1; /* Precise instruction counting */
+    /* Precise instruction counting */
+    return icount_enabled() == ICOUNT_PRECISE;
 }
 
 static uint64_t instructions_get_count(CPUARMState *env)
-- 
2.41.0

[PATCH v3 4/6] target/arm: Ensure icount is enabled when emulating INST_RETIRED

[Philippe Mathieu-Daudé]

pmu_init() register its event checking the pm_event::supported()
handler. For INST_RETIRED, the event is only registered and the
bit enabled in the PMU Common Event Identification register when
icount is enabled as ICOUNT_PRECISE.

PMU events are TCG-only, hardware accelerators handle them
directly. Unfortunately we register the events in non-TCG builds,
leading to linking error such:

  ld: Undefined symbols:
    _icount_to_ns, referenced from:
      _instructions_ns_per in target_arm_helper.c.o
  clang: error: linker command failed with exit code 1 (use -v to see invocation)

As a kludge, give a hint to the compiler by asserting the
pm_event::get_count() and pm_event::ns_per_count() handler will
only be called under this icount mode.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
As discussed in
https://lore.kernel.org/qemu-devel/CAFEAcA-HVf8vWLzmdStEo2NrSKQdZV612rBjiaj-gLW4vXyvpA@mail.gmail.com/
better would be to restrict the PMU events to TCG, but this is
out of the scope of this series.
---
 target/arm/helper.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index adb0960bba..333fd5f4bf 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -940,11 +940,13 @@ static bool instructions_supported(CPUARMState *env)
 
 static uint64_t instructions_get_count(CPUARMState *env)
 {
+    assert(icount_enabled() == ICOUNT_PRECISE);
     return (uint64_t)icount_get_raw();
 }
 
 static int64_t instructions_ns_per(uint64_t icount)
 {
+    assert(icount_enabled() == ICOUNT_PRECISE);
     return icount_to_ns((int64_t)icount);
 }
 #endif
-- 
2.41.0

[PATCH v3 5/6] util/async: Only call icount_notify_exit() if icount is enabled

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 accel/tcg/icount-common.c |  4 +++-
 stubs/icount.c            |  2 +-
 util/async.c              | 16 +++++++++-------
 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/accel/tcg/icount-common.c b/accel/tcg/icount-common.c
index f0f8fc7f1c..a4a747d1dc 100644
--- a/accel/tcg/icount-common.c
+++ b/accel/tcg/icount-common.c
@@ -495,7 +495,9 @@ bool icount_configure(QemuOpts *opts, Error **errp)
 
 void icount_notify_exit(void)
 {
-    if (icount_enabled() && current_cpu) {
+    assert(icount_enabled());
+
+    if (current_cpu) {
         qemu_cpu_kick(current_cpu);
         qemu_clock_notify(QEMU_CLOCK_VIRTUAL);
     }
diff --git a/stubs/icount.c b/stubs/icount.c
index 7055c13725..b060b03a73 100644
--- a/stubs/icount.c
+++ b/stubs/icount.c
@@ -37,7 +37,7 @@ void icount_account_warp_timer(void)
 {
     abort();
 }
-
 void icount_notify_exit(void)
 {
+    abort();
 }
diff --git a/util/async.c b/util/async.c
index 8f90ddc304..9007642c27 100644
--- a/util/async.c
+++ b/util/async.c
@@ -94,13 +94,15 @@ static void aio_bh_enqueue(QEMUBH *bh, unsigned new_flags)
     }
 
     aio_notify(ctx);
-    /*
-     * Workaround for record/replay.
-     * vCPU execution should be suspended when new BH is set.
-     * This is needed to avoid guest timeouts caused
-     * by the long cycles of the execution.
-     */
-    icount_notify_exit();
+    if (unlikely(icount_enabled())) {
+        /*
+         * Workaround for record/replay.
+         * vCPU execution should be suspended when new BH is set.
+         * This is needed to avoid guest timeouts caused
+         * by the long cycles of the execution.
+         */
+        icount_notify_exit();
+    }
 }
 
 /* Only called from aio_bh_poll() and aio_ctx_finalize() */
-- 
2.41.0

[PATCH v3 6/6] sysemu/replay: Restrict icount to system emulation

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/sysemu/cpu-timers.h |  2 +-
 include/sysemu/replay.h     | 11 ++++++++---
 stubs/icount.c              | 19 -------------------
 3 files changed, 9 insertions(+), 23 deletions(-)

diff --git a/include/sysemu/cpu-timers.h b/include/sysemu/cpu-timers.h
index 3f05f29b10..d86738a378 100644
--- a/include/sysemu/cpu-timers.h
+++ b/include/sysemu/cpu-timers.h
@@ -30,7 +30,7 @@ typedef enum {
     ICOUNT_ADAPTATIVE,
 } ICountMode;
 
-#ifdef CONFIG_TCG
+#if defined(CONFIG_TCG) && !defined(CONFIG_USER_ONLY)
 extern ICountMode use_icount;
 #define icount_enabled() (use_icount)
 #else
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 08aae5869f..8102fa54f0 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -1,6 +1,3 @@
-#ifndef SYSEMU_REPLAY_H
-#define SYSEMU_REPLAY_H
-
 /*
  * QEMU replay (system interface)
  *
@@ -11,6 +8,12 @@
  * See the COPYING file in the top-level directory.
  *
  */
+#ifndef SYSEMU_REPLAY_H
+#define SYSEMU_REPLAY_H
+
+#ifdef CONFIG_USER_ONLY
+#error Cannot include this header from user emulation
+#endif
 
 #include "exec/replay-core.h"
 #include "qapi/qapi-types-misc.h"
@@ -79,12 +82,14 @@ int64_t replay_save_clock(ReplayClockKind kind, int64_t clock,
 int64_t replay_read_clock(ReplayClockKind kind, int64_t raw_icount);
 /*! Saves or reads the clock depending on the current replay mode. */
 #define REPLAY_CLOCK(clock, value)                                      \
+    !icount_enabled() ? (value) :                                       \
     (replay_mode == REPLAY_MODE_PLAY                                    \
         ? replay_read_clock((clock), icount_get_raw())                  \
         : replay_mode == REPLAY_MODE_RECORD                             \
             ? replay_save_clock((clock), (value), icount_get_raw())     \
             : (value))
 #define REPLAY_CLOCK_LOCKED(clock, value)                               \
+    !icount_enabled() ? (value) :                                       \
     (replay_mode == REPLAY_MODE_PLAY                                    \
         ? replay_read_clock((clock), icount_get_raw_locked())           \
         : replay_mode == REPLAY_MODE_RECORD                             \
diff --git a/stubs/icount.c b/stubs/icount.c
index b060b03a73..9a29084ecc 100644
--- a/stubs/icount.c
+++ b/stubs/icount.c
@@ -5,30 +5,11 @@
 
 ICountMode use_icount = ICOUNT_DISABLED;
 
-void icount_update(CPUState *cpu)
-{
-    abort();
-}
 int64_t icount_get_raw(void)
 {
     abort();
     return 0;
 }
-int64_t icount_get(void)
-{
-    abort();
-    return 0;
-}
-int64_t icount_to_ns(int64_t icount)
-{
-    abort();
-    return 0;
-}
-int64_t icount_round(int64_t count)
-{
-    abort();
-    return 0;
-}
 void icount_start_warp_timer(void)
 {
     abort();
-- 
2.41.0

Re: [PATCH v3 3/6] sysemu/cpu-timers: Introduce ICountMode enumerator

[Philippe Mathieu-Daudé]

On 8/12/23 12:35, Philippe Mathieu-Daudé wrote:
> Rather than having to lookup for what the 0, 1, 2, ...
> icount values are, use a enum definition.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/sysemu/cpu-timers.h | 20 +++++++++++++-------
>   accel/tcg/icount-common.c   | 16 +++++++---------
>   stubs/icount.c              |  2 +-
>   system/cpu-timers.c         |  2 +-
>   target/arm/helper.c         |  3 ++-
>   5 files changed, 24 insertions(+), 19 deletions(-)
> 
> diff --git a/include/sysemu/cpu-timers.h b/include/sysemu/cpu-timers.h
> index b70dc7692d..3f05f29b10 100644
> --- a/include/sysemu/cpu-timers.h
> +++ b/include/sysemu/cpu-timers.h
> @@ -17,18 +17,24 @@ void cpu_timers_init(void);
>   
>   /* icount - Instruction Counter API */
>   
> -/*
> - * icount enablement state:
> +/**
> + * ICountMode: icount enablement state:
>    *
> - * 0 = Disabled - Do not count executed instructions.
> - * 1 = Enabled - Fixed conversion of insn to ns via "shift" option
> - * 2 = Enabled - Runtime adaptive algorithm to compute shift
> + * @ICOUNT_DISABLED: Disabled - Do not count executed instructions.
> + * @ICOUNT_PRECISE: Enabled - Fixed conversion of insn to ns via "shift" option
> + * @ICOUNT_ADAPTATIVE: Enabled - Runtime adaptive algorithm to compute shift
>    */
> +typedef enum {
> +    ICOUNT_DISABLED = 0,
> +    ICOUNT_PRECISE,
> +    ICOUNT_ADAPTATIVE,
> +} ICountMode;

Per v2:
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Re: [PATCH v3 1/6] sysemu/cpu-timers: Have icount_configure() return a boolean

[Richard Henderson]

On 12/8/23 03:35, Philippe Mathieu-Daudé wrote:
> Following the example documented since commit e3fe3988d7 ("error:
> Document Error API usage rules"), have icount_configure()
> return a boolean indicating whether an error is set or not.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   include/sysemu/cpu-timers.h | 10 ++++++++--
>   accel/tcg/icount-common.c   | 16 +++++++++-------
>   stubs/icount.c              |  4 +++-
>   system/vl.c                 |  3 +--
>   4 files changed, 21 insertions(+), 12 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v3 2/6] system/vl: Evaluate icount after accelerator options are parsed

[Richard Henderson]

On 12/8/23 03:35, Philippe Mathieu-Daudé wrote:
> We need to parse the accelerators first, to be able
> to check whether TCG is enabled or not. Then we can
> parse the -icount option.
> 
> This allows removing the icount_configure() stub.
> 
> Fixes: 7f8b6126e7 ("vl: move icount configuration earlier")
> Reported-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   stubs/icount.c |  8 --------
>   system/vl.c    | 16 +++++++++-------
>   2 files changed, 9 insertions(+), 15 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH v3 4/6] target/arm: Ensure icount is enabled when emulating INST_RETIRED

[Richard Henderson]

On 12/8/23 03:35, Philippe Mathieu-Daudé wrote:
> pmu_init() register its event checking the pm_event::supported()
> handler. For INST_RETIRED, the event is only registered and the
> bit enabled in the PMU Common Event Identification register when
> icount is enabled as ICOUNT_PRECISE.
> 
> PMU events are TCG-only, hardware accelerators handle them
> directly. Unfortunately we register the events in non-TCG builds,
> leading to linking error such:
> 
>    ld: Undefined symbols:
>      _icount_to_ns, referenced from:
>        _instructions_ns_per in target_arm_helper.c.o
>    clang: error: linker command failed with exit code 1 (use -v to see invocation)
> 
> As a kludge, give a hint to the compiler by asserting the
> pm_event::get_count() and pm_event::ns_per_count() handler will
> only be called under this icount mode.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> As discussed in
> https://lore.kernel.org/qemu-devel/CAFEAcA-HVf8vWLzmdStEo2NrSKQdZV612rBjiaj-gLW4vXyvpA@mail.gmail.com/
> better would be to restrict the PMU events to TCG, but this is
> out of the scope of this series.


Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH v3 5/6] util/async: Only call icount_notify_exit() if icount is enabled

[Richard Henderson]

On 12/8/23 03:35, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   accel/tcg/icount-common.c |  4 +++-
>   stubs/icount.c            |  2 +-
>   util/async.c              | 16 +++++++++-------
>   3 files changed, 13 insertions(+), 9 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v3 6/6] sysemu/replay: Restrict icount to system emulation

[Richard Henderson]

On 12/8/23 03:35, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   include/sysemu/cpu-timers.h |  2 +-
>   include/sysemu/replay.h     | 11 ++++++++---
>   stubs/icount.c              | 19 -------------------
>   3 files changed, 9 insertions(+), 23 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH v3 0/6] sysemu/replay: Restrict icount to TCG system emulation

[Philippe Mathieu-Daudé]

On 8/12/23 12:35, Philippe Mathieu-Daudé wrote:

> Philippe Mathieu-Daudé (6):
>    sysemu/cpu-timers: Have icount_configure() return a boolean
>    system/vl: Evaluate icount after accelerator options are parsed
>    sysemu/cpu-timers: Introduce ICountMode enumerator
>    target/arm: Ensure icount is enabled when emulating INST_RETIRED
>    util/async: Only call icount_notify_exit() if icount is enabled
>    sysemu/replay: Restrict icount to system emulation

Thanks, series queued.