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

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

[Philippe Mathieu-Daudé]

Slightly simplify non-TCG and user emulation code.

Philippe Mathieu-Daudé (3):
  util/async: Only call icount_notify_exit() if icount is enabled
  sysemu/replay: Restrict icount to TCG emulation
  sysemu/replay: Restrict icount to system emulation

 include/sysemu/cpu-timers.h |  2 +-
 include/sysemu/replay.h     | 11 ++++++++---
 stubs/icount.c              | 27 +--------------------------
 system/vl.c                 |  6 +++++-
 util/async.c                | 16 +++++++++-------
 5 files changed, 24 insertions(+), 38 deletions(-)

-- 
2.41.0

[PATCH 1/3] util/async: Only call icount_notify_exit() if icount is enabled

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 stubs/icount.c |  2 +-
 util/async.c   | 16 +++++++++-------
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/stubs/icount.c b/stubs/icount.c
index 6df8c2bf7d..fc3beac003 100644
--- a/stubs/icount.c
+++ b/stubs/icount.c
@@ -43,7 +43,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 2/3] sysemu/replay: Restrict icount to TCG emulation

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/sysemu/replay.h | 2 ++
 stubs/icount.c          | 6 ------
 system/vl.c             | 6 +++++-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 08aae5869f..02fa75c783 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -79,12 +79,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 fc3beac003..c39a65da92 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,11 +9,6 @@ void icount_update(CPUState *cpu)
 {
     abort();
 }
-void icount_configure(QemuOpts *opts, Error **errp)
-{
-    /* signal error */
-    error_setg(errp, "cannot configure icount, TCG support not available");
-}
 int64_t icount_get_raw(void)
 {
     abort();
diff --git a/system/vl.c b/system/vl.c
index 2bcd9efb9a..8c99c5f681 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -2270,7 +2270,11 @@ static void user_register_global_props(void)
 
 static int do_configure_icount(void *opaque, QemuOpts *opts, Error **errp)
 {
-    icount_configure(opts, errp);
+    if (tcg_enabled()) {
+        icount_configure(opts, errp);
+    } else {
+        error_setg(errp, "cannot configure icount, TCG support not available");
+    }
     return 0;
 }
 
-- 
2.41.0

[PATCH 3/3] 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     |  9 ++++++---
 stubs/icount.c              | 19 -------------------
 3 files changed, 7 insertions(+), 23 deletions(-)

diff --git a/include/sysemu/cpu-timers.h b/include/sysemu/cpu-timers.h
index 2e786fe7fb..188f67ee90 100644
--- a/include/sysemu/cpu-timers.h
+++ b/include/sysemu/cpu-timers.h
@@ -24,7 +24,7 @@ void cpu_timers_init(void);
  * 1 = Enabled - Fixed conversion of insn to ns via "shift" option
  * 2 = Enabled - Runtime adaptive algorithm to compute shift
  */
-#ifdef CONFIG_TCG
+#if defined(CONFIG_TCG) && !defined(CONFIG_USER_ONLY)
 extern int use_icount;
 #define icount_enabled() (use_icount)
 #else
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 02fa75c783..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"
diff --git a/stubs/icount.c b/stubs/icount.c
index c39a65da92..ec8d150069 100644
--- a/stubs/icount.c
+++ b/stubs/icount.c
@@ -5,30 +5,11 @@
 
 int use_icount;
 
-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 3/3] sysemu/replay: Restrict icount to system emulation

[Philippe Mathieu-Daudé]

On 7/12/23 11:26, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/sysemu/cpu-timers.h |  2 +-
>   include/sysemu/replay.h     |  9 ++++++---
>   stubs/icount.c              | 19 -------------------
>   3 files changed, 7 insertions(+), 23 deletions(-)
> 
> diff --git a/include/sysemu/cpu-timers.h b/include/sysemu/cpu-timers.h
> index 2e786fe7fb..188f67ee90 100644
> --- a/include/sysemu/cpu-timers.h
> +++ b/include/sysemu/cpu-timers.h
> @@ -24,7 +24,7 @@ void cpu_timers_init(void);
>    * 1 = Enabled - Fixed conversion of insn to ns via "shift" option
>    * 2 = Enabled - Runtime adaptive algorithm to compute shift
>    */
> -#ifdef CONFIG_TCG
> +#if defined(CONFIG_TCG) && !defined(CONFIG_USER_ONLY)
>   extern int use_icount;
>   #define icount_enabled() (use_icount)
>   #else
> diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
> index 02fa75c783..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"
> diff --git a/stubs/icount.c b/stubs/icount.c
> index c39a65da92..ec8d150069 100644
> --- a/stubs/icount.c
> +++ b/stubs/icount.c
> @@ -5,30 +5,11 @@
>   
>   int use_icount;
>   
> -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;
> -}

Build failure on HVF due to:

   pmu_init()
    -> pm_events[]
     -> INST_RETIRED
      -> instructions_ns_per()
       -> icount_to_ns()

So we need to keep the icount_to_ns() stub until we restrict
ARM PMU code to TCG.

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

[Philippe Mathieu-Daudé]

On 7/12/23 14:46, Philippe Mathieu-Daudé wrote:
> On 7/12/23 11:26, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   include/sysemu/cpu-timers.h |  2 +-
>>   include/sysemu/replay.h     |  9 ++++++---
>>   stubs/icount.c              | 19 -------------------
>>   3 files changed, 7 insertions(+), 23 deletions(-)
>>
>> diff --git a/include/sysemu/cpu-timers.h b/include/sysemu/cpu-timers.h
>> index 2e786fe7fb..188f67ee90 100644
>> --- a/include/sysemu/cpu-timers.h
>> +++ b/include/sysemu/cpu-timers.h
>> @@ -24,7 +24,7 @@ void cpu_timers_init(void);
>>    * 1 = Enabled - Fixed conversion of insn to ns via "shift" option
>>    * 2 = Enabled - Runtime adaptive algorithm to compute shift
>>    */
>> -#ifdef CONFIG_TCG
>> +#if defined(CONFIG_TCG) && !defined(CONFIG_USER_ONLY)
>>   extern int use_icount;
>>   #define icount_enabled() (use_icount)
>>   #else
>> diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
>> index 02fa75c783..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"
>> diff --git a/stubs/icount.c b/stubs/icount.c
>> index c39a65da92..ec8d150069 100644
>> --- a/stubs/icount.c
>> +++ b/stubs/icount.c
>> @@ -5,30 +5,11 @@
>>   int use_icount;
>> -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;
>> -}
> 
> Build failure on HVF due to:
> 
>    pmu_init()
>     -> pm_events[]
>      -> INST_RETIRED
>       -> instructions_ns_per()
>        -> icount_to_ns()
> 
> So we need to keep the icount_to_ns() stub until we restrict
> ARM PMU code to TCG.

Alternatively, we can use as a preliminary patch:

-- >8 --
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 20e13215bb..48ab1e0523 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -939,11 +939,13 @@ static bool instructions_supported(CPUARMState *env)

  static uint64_t instructions_get_count(CPUARMState *env)
  {
+    assert(icount_enabled());
      return (uint64_t)icount_get_raw();
  }

  static int64_t instructions_ns_per(uint64_t icount)
  {
+    assert(icount_enabled());
      return icount_to_ns((int64_t)icount);
  }
  #endif
---