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
---