[1/1] allow using a higher icount

Commit Message

Elisha Hollander Aug. 16, 2024, 4:20 p.m. UTC

Signed-off-by: Elisha Hollander <[e-mail]>
---
 accel/tcg/cpu-exec.c      | 4 +---
 accel/tcg/icount-common.c | 4 ++--
 2 files changed, 3 insertions(+), 5 deletions(-)

Comments

Elisha Hollander Aug. 24, 2024, 7:46 p.m. UTC | #1

Just wanted to ping the patch
<https://lore.kernel.org/qemu-devel/[e-mail]/>

On Fri, Aug 16, 2024, 19:21 Elisha Hollander <[e-mail]>
wrote:

> Signed-off-by: Elisha Hollander <[e-mail]>
> ---
>  accel/tcg/cpu-exec.c      | 4 +---
>  accel/tcg/icount-common.c | 4 ++--
>  2 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 8163295f34..4c2baf8ed4 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -95,11 +95,10 @@ static void align_clocks(SyncClocks *sc, CPUState *cpu)
>  static void print_delay(const SyncClocks *sc)
>  {
>      static float threshold_delay;
> -    static int64_t last_realtime_clock;
>      static int nb_prints;
>
>      if (icount_align_option &&
> -        sc->realtime_clock - last_realtime_clock >= MAX_DELAY_PRINT_RATE
> &&
> +        sc->diff_clk >= MAX_DELAY_PRINT_RATE &&
>          nb_prints < MAX_NB_PRINTS) {
>          if ((-sc->diff_clk / (float)1000000000LL > threshold_delay) ||
>              (-sc->diff_clk / (float)1000000000LL <
> @@ -109,7 +108,6 @@ static void print_delay(const SyncClocks *sc)
>                          threshold_delay - 1,
>                          threshold_delay);
>              nb_prints++;
> -            last_realtime_clock = sc->realtime_clock;
>          }
>      }
>  }
> diff --git a/accel/tcg/icount-common.c b/accel/tcg/icount-common.c
> index 8d3d3a7e9d..f07f8baf4d 100644
> --- a/accel/tcg/icount-common.c
> +++ b/accel/tcg/icount-common.c
> @@ -46,8 +46,8 @@
>   * is TCG-specific, and does not need to be built for other accels.
>   */
>  static bool icount_sleep = true;
> -/* Arbitrarily pick 1MIPS as the minimum allowable speed.  */
> -#define MAX_ICOUNT_SHIFT 10
> +/* Arbitrarily pick the minimum allowable speed.  */
> +#define MAX_ICOUNT_SHIFT 30
>
>  /* Do not count executed instructions */
>  ICountMode use_icount = ICOUNT_DISABLED;
> --
> 2.30.2
>
>

Alex Bennée Aug. 27, 2024, 9:38 a.m. UTC | #2

Elisha Hollander <[e-mail]> writes:

> Signed-off-by: Elisha Hollander <[e-mail]>

What is the use-case for this patch?

If you are simply looking to slow the emulated system down please have a
look at:

  https://qemu.readthedocs.io/en/master/about/emulation.html#limit-instructions-per-second

which uses the plugin system to limit the run rate and sleep if its
running too fast. The longer term goal is to deprecate the icount clock
alignment feature from the core code and leave icount to just provide
the deterministic execution needed for record/replay and reverse
debugging.


> ---
>  accel/tcg/cpu-exec.c      | 4 +---
>  accel/tcg/icount-common.c | 4 ++--
>  2 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 8163295f34..4c2baf8ed4 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -95,11 +95,10 @@ static void align_clocks(SyncClocks *sc, CPUState *cpu)
>  static void print_delay(const SyncClocks *sc)
>  {
>      static float threshold_delay;
> -    static int64_t last_realtime_clock;
>      static int nb_prints;
>  
>      if (icount_align_option &&
> -        sc->realtime_clock - last_realtime_clock >= MAX_DELAY_PRINT_RATE &&
> +        sc->diff_clk >= MAX_DELAY_PRINT_RATE &&
>          nb_prints < MAX_NB_PRINTS) {
>          if ((-sc->diff_clk / (float)1000000000LL > threshold_delay) ||
>              (-sc->diff_clk / (float)1000000000LL <
> @@ -109,7 +108,6 @@ static void print_delay(const SyncClocks *sc)
>                          threshold_delay - 1,
>                          threshold_delay);
>              nb_prints++;
> -            last_realtime_clock = sc->realtime_clock;
>          }
>      }
>  }
> diff --git a/accel/tcg/icount-common.c b/accel/tcg/icount-common.c
> index 8d3d3a7e9d..f07f8baf4d 100644
> --- a/accel/tcg/icount-common.c
> +++ b/accel/tcg/icount-common.c
> @@ -46,8 +46,8 @@
>   * is TCG-specific, and does not need to be built for other accels.
>   */
>  static bool icount_sleep = true;
> -/* Arbitrarily pick 1MIPS as the minimum allowable speed.  */
> -#define MAX_ICOUNT_SHIFT 10
> +/* Arbitrarily pick the minimum allowable speed.  */
> +#define MAX_ICOUNT_SHIFT 30
>  
>  /* Do not count executed instructions */
>  ICountMode use_icount = ICOUNT_DISABLED;

Elisha Hollander Aug. 27, 2024, 11:59 a.m. UTC | #3

Oh nice, I didn't know that

On Tue, Aug 27, 2024, 12:39 Alex Bennée <[e-mail]> wrote:

> Elisha Hollander <[e-mail]> writes:
>
> > Signed-off-by: Elisha Hollander <[e-mail]>
>
> What is the use-case for this patch?
>
> If you are simply looking to slow the emulated system down please have a
> look at:
>
>
> https://qemu.readthedocs.io/en/master/about/emulation.html#limit-instructions-per-second
>
> which uses the plugin system to limit the run rate and sleep if its
> running too fast. The longer term goal is to deprecate the icount clock
> alignment feature from the core code and leave icount to just provide
> the deterministic execution needed for record/replay and reverse
> debugging.
>
>
> > ---
> >  accel/tcg/cpu-exec.c      | 4 +---
> >  accel/tcg/icount-common.c | 4 ++--
> >  2 files changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> > index 8163295f34..4c2baf8ed4 100644
> > --- a/accel/tcg/cpu-exec.c
> > +++ b/accel/tcg/cpu-exec.c
> > @@ -95,11 +95,10 @@ static void align_clocks(SyncClocks *sc, CPUState
> *cpu)
> >  static void print_delay(const SyncClocks *sc)
> >  {
> >      static float threshold_delay;
> > -    static int64_t last_realtime_clock;
> >      static int nb_prints;
> >
> >      if (icount_align_option &&
> > -        sc->realtime_clock - last_realtime_clock >=
> MAX_DELAY_PRINT_RATE &&
> > +        sc->diff_clk >= MAX_DELAY_PRINT_RATE &&
> >          nb_prints < MAX_NB_PRINTS) {
> >          if ((-sc->diff_clk / (float)1000000000LL > threshold_delay) ||
> >              (-sc->diff_clk / (float)1000000000LL <
> > @@ -109,7 +108,6 @@ static void print_delay(const SyncClocks *sc)
> >                          threshold_delay - 1,
> >                          threshold_delay);
> >              nb_prints++;
> > -            last_realtime_clock = sc->realtime_clock;
> >          }
> >      }
> >  }
> > diff --git a/accel/tcg/icount-common.c b/accel/tcg/icount-common.c
> > index 8d3d3a7e9d..f07f8baf4d 100644
> > --- a/accel/tcg/icount-common.c
> > +++ b/accel/tcg/icount-common.c
> > @@ -46,8 +46,8 @@
> >   * is TCG-specific, and does not need to be built for other accels.
> >   */
> >  static bool icount_sleep = true;
> > -/* Arbitrarily pick 1MIPS as the minimum allowable speed.  */
> > -#define MAX_ICOUNT_SHIFT 10
> > +/* Arbitrarily pick the minimum allowable speed.  */
> > +#define MAX_ICOUNT_SHIFT 30
> >
> >  /* Do not count executed instructions */
> >  ICountMode use_icount = ICOUNT_DISABLED;
>
> --
> Alex Bennée
> Virtualisation Tech Lead @ Linaro
>

Elisha Hollander Aug. 28, 2024, 10:36 a.m. UTC | #4

Although it gives `undefined symbol: qemu_plugin_scoreboard_free`. But
probably I messed something up...

On Tue, Aug 27, 2024, 14:59 Elisha Hollander <[e-mail]>
wrote:

> Oh nice, I didn't know that
>
> On Tue, Aug 27, 2024, 12:39 Alex Bennée <[e-mail]> wrote:
>
>> Elisha Hollander <[e-mail]> writes:
>>
>> > Signed-off-by: Elisha Hollander <[e-mail]>
>>
>> What is the use-case for this patch?
>>
>> If you are simply looking to slow the emulated system down please have a
>> look at:
>>
>>
>> https://qemu.readthedocs.io/en/master/about/emulation.html#limit-instructions-per-second
>>
>> which uses the plugin system to limit the run rate and sleep if its
>> running too fast. The longer term goal is to deprecate the icount clock
>> alignment feature from the core code and leave icount to just provide
>> the deterministic execution needed for record/replay and reverse
>> debugging.
>>
>>
>> > ---
>> >  accel/tcg/cpu-exec.c      | 4 +---
>> >  accel/tcg/icount-common.c | 4 ++--
>> >  2 files changed, 3 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
>> > index 8163295f34..4c2baf8ed4 100644
>> > --- a/accel/tcg/cpu-exec.c
>> > +++ b/accel/tcg/cpu-exec.c
>> > @@ -95,11 +95,10 @@ static void align_clocks(SyncClocks *sc, CPUState
>> *cpu)
>> >  static void print_delay(const SyncClocks *sc)
>> >  {
>> >      static float threshold_delay;
>> > -    static int64_t last_realtime_clock;
>> >      static int nb_prints;
>> >
>> >      if (icount_align_option &&
>> > -        sc->realtime_clock - last_realtime_clock >=
>> MAX_DELAY_PRINT_RATE &&
>> > +        sc->diff_clk >= MAX_DELAY_PRINT_RATE &&
>> >          nb_prints < MAX_NB_PRINTS) {
>> >          if ((-sc->diff_clk / (float)1000000000LL > threshold_delay) ||
>> >              (-sc->diff_clk / (float)1000000000LL <
>> > @@ -109,7 +108,6 @@ static void print_delay(const SyncClocks *sc)
>> >                          threshold_delay - 1,
>> >                          threshold_delay);
>> >              nb_prints++;
>> > -            last_realtime_clock = sc->realtime_clock;
>> >          }
>> >      }
>> >  }
>> > diff --git a/accel/tcg/icount-common.c b/accel/tcg/icount-common.c
>> > index 8d3d3a7e9d..f07f8baf4d 100644
>> > --- a/accel/tcg/icount-common.c
>> > +++ b/accel/tcg/icount-common.c
>> > @@ -46,8 +46,8 @@
>> >   * is TCG-specific, and does not need to be built for other accels.
>> >   */
>> >  static bool icount_sleep = true;
>> > -/* Arbitrarily pick 1MIPS as the minimum allowable speed.  */
>> > -#define MAX_ICOUNT_SHIFT 10
>> > +/* Arbitrarily pick the minimum allowable speed.  */
>> > +#define MAX_ICOUNT_SHIFT 30
>> >
>> >  /* Do not count executed instructions */
>> >  ICountMode use_icount = ICOUNT_DISABLED;
>>
>> --
>> Alex Bennée
>> Virtualisation Tech Lead @ Linaro
>>
>

Alex Bennée Aug. 28, 2024, 11:05 a.m. UTC | #5

Elisha Hollander <[e-mail]> writes:

> Although it gives `undefined symbol: qemu_plugin_scoreboard_free`. But
> probably I messed something up...

Are you using an older QEMU? We should trigger an API warning if they
are mismatched but maybe thats not working.

>
> On Tue, Aug 27, 2024, 14:59 Elisha Hollander <[e-mail]> wrote:
>
>  Oh nice, I didn't know that
>
>  On Tue, Aug 27, 2024, 12:39 Alex Bennée <[e-mail]> wrote:
>
>  Elisha Hollander <[e-mail]> writes:
>
>  > Signed-off-by: Elisha Hollander <[e-mail]>
>
>  What is the use-case for this patch?
>
>  If you are simply looking to slow the emulated system down please have a
>  look at:
>
>    https://qemu.readthedocs.io/en/master/about/emulation.html#limit-instructions-per-second
>
>  which uses the plugin system to limit the run rate and sleep if its
>  running too fast. The longer term goal is to deprecate the icount clock
>  alignment feature from the core code and leave icount to just provide
>  the deterministic execution needed for record/replay and reverse
>  debugging.
>
>  > ---
>  >  accel/tcg/cpu-exec.c      | 4 +---
>  >  accel/tcg/icount-common.c | 4 ++--
>  >  2 files changed, 3 insertions(+), 5 deletions(-)
>  >
>  > diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
>  > index 8163295f34..4c2baf8ed4 100644
>  > --- a/accel/tcg/cpu-exec.c
>  > +++ b/accel/tcg/cpu-exec.c
>  > @@ -95,11 +95,10 @@ static void align_clocks(SyncClocks *sc, CPUState *cpu)
>  >  static void print_delay(const SyncClocks *sc)
>  >  {
>  >      static float threshold_delay;
>  > -    static int64_t last_realtime_clock;
>  >      static int nb_prints;
>  >  
>  >      if (icount_align_option &&
>  > -        sc->realtime_clock - last_realtime_clock >= MAX_DELAY_PRINT_RATE &&
>  > +        sc->diff_clk >= MAX_DELAY_PRINT_RATE &&
>  >          nb_prints < MAX_NB_PRINTS) {
>  >          if ((-sc->diff_clk / (float)1000000000LL > threshold_delay) ||
>  >              (-sc->diff_clk / (float)1000000000LL <
>  > @@ -109,7 +108,6 @@ static void print_delay(const SyncClocks *sc)
>  >                          threshold_delay - 1,
>  >                          threshold_delay);
>  >              nb_prints++;
>  > -            last_realtime_clock = sc->realtime_clock;
>  >          }
>  >      }
>  >  }
>  > diff --git a/accel/tcg/icount-common.c b/accel/tcg/icount-common.c
>  > index 8d3d3a7e9d..f07f8baf4d 100644
>  > --- a/accel/tcg/icount-common.c
>  > +++ b/accel/tcg/icount-common.c
>  > @@ -46,8 +46,8 @@
>  >   * is TCG-specific, and does not need to be built for other accels.
>  >   */
>  >  static bool icount_sleep = true;
>  > -/* Arbitrarily pick 1MIPS as the minimum allowable speed.  */
>  > -#define MAX_ICOUNT_SHIFT 10
>  > +/* Arbitrarily pick the minimum allowable speed.  */
>  > +#define MAX_ICOUNT_SHIFT 30
>  >  
>  >  /* Do not count executed instructions */
>  >  ICountMode use_icount = ICOUNT_DISABLED;
>
>  -- 
>  Alex Bennée
>  Virtualisation Tech Lead @ Linaro

Elisha Hollander Sept. 2, 2024, 12:38 p.m. UTC | #6

Just checked with 9.0.2 it it still gives the error...

On Wed, Aug 28, 2024, 14:05 Alex Bennée <[e-mail]> wrote:

> Elisha Hollander <[e-mail]> writes:
>
> > Although it gives `undefined symbol: qemu_plugin_scoreboard_free`. But
> > probably I messed something up...
>
> Are you using an older QEMU? We should trigger an API warning if they
> are mismatched but maybe thats not working.
>
> >
> > On Tue, Aug 27, 2024, 14:59 Elisha Hollander <[e-mail]>
> wrote:
> >
> >  Oh nice, I didn't know that
> >
> >  On Tue, Aug 27, 2024, 12:39 Alex Bennée <[e-mail]> wrote:
> >
> >  Elisha Hollander <[e-mail]> writes:
> >
> >  > Signed-off-by: Elisha Hollander <[e-mail]>
> >
> >  What is the use-case for this patch?
> >
> >  If you are simply looking to slow the emulated system down please have a
> >  look at:
> >
> >
> https://qemu.readthedocs.io/en/master/about/emulation.html#limit-instructions-per-second
> >
> >  which uses the plugin system to limit the run rate and sleep if its
> >  running too fast. The longer term goal is to deprecate the icount clock
> >  alignment feature from the core code and leave icount to just provide
> >  the deterministic execution needed for record/replay and reverse
> >  debugging.
> >
> >  > ---
> >  >  accel/tcg/cpu-exec.c      | 4 +---
> >  >  accel/tcg/icount-common.c | 4 ++--
> >  >  2 files changed, 3 insertions(+), 5 deletions(-)
> >  >
> >  > diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> >  > index 8163295f34..4c2baf8ed4 100644
> >  > --- a/accel/tcg/cpu-exec.c
> >  > +++ b/accel/tcg/cpu-exec.c
> >  > @@ -95,11 +95,10 @@ static void align_clocks(SyncClocks *sc, CPUState
> *cpu)
> >  >  static void print_delay(const SyncClocks *sc)
> >  >  {
> >  >      static float threshold_delay;
> >  > -    static int64_t last_realtime_clock;
> >  >      static int nb_prints;
> >  >
> >  >      if (icount_align_option &&
> >  > -        sc->realtime_clock - last_realtime_clock >=
> MAX_DELAY_PRINT_RATE &&
> >  > +        sc->diff_clk >= MAX_DELAY_PRINT_RATE &&
> >  >          nb_prints < MAX_NB_PRINTS) {
> >  >          if ((-sc->diff_clk / (float)1000000000LL > threshold_delay)
> ||
> >  >              (-sc->diff_clk / (float)1000000000LL <
> >  > @@ -109,7 +108,6 @@ static void print_delay(const SyncClocks *sc)
> >  >                          threshold_delay - 1,
> >  >                          threshold_delay);
> >  >              nb_prints++;
> >  > -            last_realtime_clock = sc->realtime_clock;
> >  >          }
> >  >      }
> >  >  }
> >  > diff --git a/accel/tcg/icount-common.c b/accel/tcg/icount-common.c
> >  > index 8d3d3a7e9d..f07f8baf4d 100644
> >  > --- a/accel/tcg/icount-common.c
> >  > +++ b/accel/tcg/icount-common.c
> >  > @@ -46,8 +46,8 @@
> >  >   * is TCG-specific, and does not need to be built for other accels.
> >  >   */
> >  >  static bool icount_sleep = true;
> >  > -/* Arbitrarily pick 1MIPS as the minimum allowable speed.  */
> >  > -#define MAX_ICOUNT_SHIFT 10
> >  > +/* Arbitrarily pick the minimum allowable speed.  */
> >  > +#define MAX_ICOUNT_SHIFT 30
> >  >
> >  >  /* Do not count executed instructions */
> >  >  ICountMode use_icount = ICOUNT_DISABLED;
> >
> >  --
> >  Alex Bennée
> >  Virtualisation Tech Lead @ Linaro
>
> --
> Alex Bennée
> Virtualisation Tech Lead @ Linaro
>

Elisha Hollander Sept. 2, 2024, 12:38 p.m. UTC | #7

But for qemu_plugin_update_ns

On Mon, Sep 2, 2024, 15:38 Elisha Hollander <[e-mail]>
wrote:

> Just checked with 9.0.2 it it still gives the error...
>
> On Wed, Aug 28, 2024, 14:05 Alex Bennée <[e-mail]> wrote:
>
>> Elisha Hollander <[e-mail]> writes:
>>
>> > Although it gives `undefined symbol: qemu_plugin_scoreboard_free`. But
>> > probably I messed something up...
>>
>> Are you using an older QEMU? We should trigger an API warning if they
>> are mismatched but maybe thats not working.
>>
>> >
>> > On Tue, Aug 27, 2024, 14:59 Elisha Hollander <[e-mail]>
>> wrote:
>> >
>> >  Oh nice, I didn't know that
>> >
>> >  On Tue, Aug 27, 2024, 12:39 Alex Bennée <[e-mail]>
>> wrote:
>> >
>> >  Elisha Hollander <[e-mail]> writes:
>> >
>> >  > Signed-off-by: Elisha Hollander <[e-mail]>
>> >
>> >  What is the use-case for this patch?
>> >
>> >  If you are simply looking to slow the emulated system down please have
>> a
>> >  look at:
>> >
>> >
>> https://qemu.readthedocs.io/en/master/about/emulation.html#limit-instructions-per-second
>> >
>> >  which uses the plugin system to limit the run rate and sleep if its
>> >  running too fast. The longer term goal is to deprecate the icount clock
>> >  alignment feature from the core code and leave icount to just provide
>> >  the deterministic execution needed for record/replay and reverse
>> >  debugging.
>> >
>> >  > ---
>> >  >  accel/tcg/cpu-exec.c      | 4 +---
>> >  >  accel/tcg/icount-common.c | 4 ++--
>> >  >  2 files changed, 3 insertions(+), 5 deletions(-)
>> >  >
>> >  > diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
>> >  > index 8163295f34..4c2baf8ed4 100644
>> >  > --- a/accel/tcg/cpu-exec.c
>> >  > +++ b/accel/tcg/cpu-exec.c
>> >  > @@ -95,11 +95,10 @@ static void align_clocks(SyncClocks *sc,
>> CPUState *cpu)
>> >  >  static void print_delay(const SyncClocks *sc)
>> >  >  {
>> >  >      static float threshold_delay;
>> >  > -    static int64_t last_realtime_clock;
>> >  >      static int nb_prints;
>> >  >
>> >  >      if (icount_align_option &&
>> >  > -        sc->realtime_clock - last_realtime_clock >=
>> MAX_DELAY_PRINT_RATE &&
>> >  > +        sc->diff_clk >= MAX_DELAY_PRINT_RATE &&
>> >  >          nb_prints < MAX_NB_PRINTS) {
>> >  >          if ((-sc->diff_clk / (float)1000000000LL > threshold_delay)
>> ||
>> >  >              (-sc->diff_clk / (float)1000000000LL <
>> >  > @@ -109,7 +108,6 @@ static void print_delay(const SyncClocks *sc)
>> >  >                          threshold_delay - 1,
>> >  >                          threshold_delay);
>> >  >              nb_prints++;
>> >  > -            last_realtime_clock = sc->realtime_clock;
>> >  >          }
>> >  >      }
>> >  >  }
>> >  > diff --git a/accel/tcg/icount-common.c b/accel/tcg/icount-common.c
>> >  > index 8d3d3a7e9d..f07f8baf4d 100644
>> >  > --- a/accel/tcg/icount-common.c
>> >  > +++ b/accel/tcg/icount-common.c
>> >  > @@ -46,8 +46,8 @@
>> >  >   * is TCG-specific, and does not need to be built for other accels.
>> >  >   */
>> >  >  static bool icount_sleep = true;
>> >  > -/* Arbitrarily pick 1MIPS as the minimum allowable speed.  */
>> >  > -#define MAX_ICOUNT_SHIFT 10
>> >  > +/* Arbitrarily pick the minimum allowable speed.  */
>> >  > +#define MAX_ICOUNT_SHIFT 30
>> >  >
>> >  >  /* Do not count executed instructions */
>> >  >  ICountMode use_icount = ICOUNT_DISABLED;
>> >
>> >  --
>> >  Alex Bennée
>> >  Virtualisation Tech Lead @ Linaro
>>
>> --
>> Alex Bennée
>> Virtualisation Tech Lead @ Linaro
>>
>

Alex Bennée Sept. 2, 2024, 1:08 p.m. UTC | #8

Elisha Hollander <[e-mail]> writes:

> But for qemu_plugin_update_ns
>
> On Mon, Sep 2, 2024, 15:38 Elisha Hollander <[e-mail]> wrote:
>
>  Just checked with 9.0.2 it it still gives the error...
>
>  On Wed, Aug 28, 2024, 14:05 Alex Bennée <[e-mail]> wrote:
>
>  Elisha Hollander <[e-mail]> writes:
>
>  > Although it gives `undefined symbol: qemu_plugin_scoreboard_free`. But
>  > probably I messed something up...
>
>  Are you using an older QEMU? We should trigger an API warning if they
>  are mismatched but maybe thats not working.
>
>  >
>  > On Tue, Aug 27, 2024, 14:59 Elisha Hollander <[e-mail]> wrote:
>  >
>  >  Oh nice, I didn't know that
>  >
>  >  On Tue, Aug 27, 2024, 12:39 Alex Bennée <[e-mail]> wrote:
>  >
>  >  Elisha Hollander <[e-mail]> writes:
>  >
>  >  > Signed-off-by: Elisha Hollander <[e-mail]>
>  >
>  >  What is the use-case for this patch?
>  >
>  >  If you are simply looking to slow the emulated system down please have a
>  >  look at:
>  >
>  >    https://qemu.readthedocs.io/en/master/about/emulation.html#limit-instructions-per-second
>  >
>  >  which uses the plugin system to limit the run rate and sleep if its
>  >  running too fast. The longer term goal is to deprecate the icount clock
>  >  alignment feature from the core code and leave icount to just provide
>  >  the deterministic execution needed for record/replay and reverse
>  >  debugging.
>  >
>  >  > ---
>  >  >  accel/tcg/cpu-exec.c      | 4 +---
>  >  >  accel/tcg/icount-common.c | 4 ++--
>  >  >  2 files changed, 3 insertions(+), 5 deletions(-)
>  >  >
>  >  > diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
>  >  > index 8163295f34..4c2baf8ed4 100644
>  >  > --- a/accel/tcg/cpu-exec.c
>  >  > +++ b/accel/tcg/cpu-exec.c
>  >  > @@ -95,11 +95,10 @@ static void align_clocks(SyncClocks *sc, CPUState *cpu)
>  >  >  static void print_delay(const SyncClocks *sc)
>  >  >  {
>  >  >      static float threshold_delay;
>  >  > -    static int64_t last_realtime_clock;
>  >  >      static int nb_prints;
>  >  >  
>  >  >      if (icount_align_option &&
>  >  > -        sc->realtime_clock - last_realtime_clock >= MAX_DELAY_PRINT_RATE &&
>  >  > +        sc->diff_clk >= MAX_DELAY_PRINT_RATE &&
>  >  >          nb_prints < MAX_NB_PRINTS) {
>  >  >          if ((-sc->diff_clk / (float)1000000000LL > threshold_delay) ||
>  >  >              (-sc->diff_clk / (float)1000000000LL <
>  >  > @@ -109,7 +108,6 @@ static void print_delay(const SyncClocks *sc)
>  >  >                          threshold_delay - 1,
>  >  >                          threshold_delay);
>  >  >              nb_prints++;
>  >  > -            last_realtime_clock = sc->realtime_clock;
>  >  >          }
>  >  >      }
>  >  >  }
>  >  > diff --git a/accel/tcg/icount-common.c b/accel/tcg/icount-common.c
>  >  > index 8d3d3a7e9d..f07f8baf4d 100644
>  >  > --- a/accel/tcg/icount-common.c
>  >  > +++ b/accel/tcg/icount-common.c
>  >  > @@ -46,8 +46,8 @@
>  >  >   * is TCG-specific, and does not need to be built for other accels.
>  >  >   */
>  >  >  static bool icount_sleep = true;
>  >  > -/* Arbitrarily pick 1MIPS as the minimum allowable speed.  */
>  >  > -#define MAX_ICOUNT_SHIFT 10
>  >  > +/* Arbitrarily pick the minimum allowable speed.  */
>  >  > +#define MAX_ICOUNT_SHIFT 30
>  >  >  
>  >  >  /* Do not count executed instructions */
>  >  >  ICountMode use_icount = ICOUNT_DISABLED;
>  >
>  >  -- 
>  >  Alex Bennée
>  >  Virtualisation Tech Lead @ Linaro
>
>  -- 
>  Alex Bennée
>  Virtualisation Tech Lead @ Linaro

Can you give me your command line please?

Elisha Hollander Sept. 3, 2024, 7:06 a.m. UTC | #9

Sure!

`build/qemu-system-i386 -plugin build/contrib/plugins/libips.so,ips=1
-display curses -bios bios.raw`

Also, I just tested with 9.1.0-rc4 and it resulted with
`tcg.c:3167:remove_label_use: code should not be reached`

On Mon, Sep 2, 2024, 16:08 Alex Bennée <[e-mail]> wrote:

> Elisha Hollander <[e-mail]> writes:
>
> > But for qemu_plugin_update_ns
> >
> > On Mon, Sep 2, 2024, 15:38 Elisha Hollander <[e-mail]>
> wrote:
> >
> >  Just checked with 9.0.2 it it still gives the error...
> >
> >  On Wed, Aug 28, 2024, 14:05 Alex Bennée <[e-mail]> wrote:
> >
> >  Elisha Hollander <[e-mail]> writes:
> >
> >  > Although it gives `undefined symbol: qemu_plugin_scoreboard_free`. But
> >  > probably I messed something up...
> >
> >  Are you using an older QEMU? We should trigger an API warning if they
> >  are mismatched but maybe thats not working.
> >
> >  >
> >  > On Tue, Aug 27, 2024, 14:59 Elisha Hollander <
> [e-mail]> wrote:
> >  >
> >  >  Oh nice, I didn't know that
> >  >
> >  >  On Tue, Aug 27, 2024, 12:39 Alex Bennée <[e-mail]>
> wrote:
> >  >
> >  >  Elisha Hollander <[e-mail]> writes:
> >  >
> >  >  > Signed-off-by: Elisha Hollander <[e-mail]>
> >  >
> >  >  What is the use-case for this patch?
> >  >
> >  >  If you are simply looking to slow the emulated system down please
> have a
> >  >  look at:
> >  >
> >  >
> https://qemu.readthedocs.io/en/master/about/emulation.html#limit-instructions-per-second
> >  >
> >  >  which uses the plugin system to limit the run rate and sleep if its
> >  >  running too fast. The longer term goal is to deprecate the icount
> clock
> >  >  alignment feature from the core code and leave icount to just provide
> >  >  the deterministic execution needed for record/replay and reverse
> >  >  debugging.
> >  >
> >  >  > ---
> >  >  >  accel/tcg/cpu-exec.c      | 4 +---
> >  >  >  accel/tcg/icount-common.c | 4 ++--
> >  >  >  2 files changed, 3 insertions(+), 5 deletions(-)
> >  >  >
> >  >  > diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> >  >  > index 8163295f34..4c2baf8ed4 100644
> >  >  > --- a/accel/tcg/cpu-exec.c
> >  >  > +++ b/accel/tcg/cpu-exec.c
> >  >  > @@ -95,11 +95,10 @@ static void align_clocks(SyncClocks *sc,
> CPUState *cpu)
> >  >  >  static void print_delay(const SyncClocks *sc)
> >  >  >  {
> >  >  >      static float threshold_delay;
> >  >  > -    static int64_t last_realtime_clock;
> >  >  >      static int nb_prints;
> >  >  >
> >  >  >      if (icount_align_option &&
> >  >  > -        sc->realtime_clock - last_realtime_clock >=
> MAX_DELAY_PRINT_RATE &&
> >  >  > +        sc->diff_clk >= MAX_DELAY_PRINT_RATE &&
> >  >  >          nb_prints < MAX_NB_PRINTS) {
> >  >  >          if ((-sc->diff_clk / (float)1000000000LL >
> threshold_delay) ||
> >  >  >              (-sc->diff_clk / (float)1000000000LL <
> >  >  > @@ -109,7 +108,6 @@ static void print_delay(const SyncClocks *sc)
> >  >  >                          threshold_delay - 1,
> >  >  >                          threshold_delay);
> >  >  >              nb_prints++;
> >  >  > -            last_realtime_clock = sc->realtime_clock;
> >  >  >          }
> >  >  >      }
> >  >  >  }
> >  >  > diff --git a/accel/tcg/icount-common.c b/accel/tcg/icount-common.c
> >  >  > index 8d3d3a7e9d..f07f8baf4d 100644
> >  >  > --- a/accel/tcg/icount-common.c
> >  >  > +++ b/accel/tcg/icount-common.c
> >  >  > @@ -46,8 +46,8 @@
> >  >  >   * is TCG-specific, and does not need to be built for other
> accels.
> >  >  >   */
> >  >  >  static bool icount_sleep = true;
> >  >  > -/* Arbitrarily pick 1MIPS as the minimum allowable speed.  */
> >  >  > -#define MAX_ICOUNT_SHIFT 10
> >  >  > +/* Arbitrarily pick the minimum allowable speed.  */
> >  >  > +#define MAX_ICOUNT_SHIFT 30
> >  >  >
> >  >  >  /* Do not count executed instructions */
> >  >  >  ICountMode use_icount = ICOUNT_DISABLED;
> >  >
> >  >  --
> >  >  Alex Bennée
> >  >  Virtualisation Tech Lead @ Linaro
> >
> >  --
> >  Alex Bennée
> >  Virtualisation Tech Lead @ Linaro
>
> Can you give me your command line please?
>
> --
> Alex Bennée
> Virtualisation Tech Lead @ Linaro
>

Alex Bennée Sept. 3, 2024, 10:21 a.m. UTC | #10

Elisha Hollander <[e-mail]> writes:

> Sure!
>
> `build/qemu-system-i386 -plugin build/contrib/plugins/libips.so,ips=1 -display curses -bios bios.raw`
>
> Also, I just tested with 9.1.0-rc4 and it resulted with
> `tcg.c:3167:remove_label_use: code should not be reached`

I can replicate but it seems to be an edge case with ips=1, try a higher
number, even ips=10 doesn't trigger the crash (but 10 instructions a
second is very slow to do anything).

  ./qemu-system-i386  -plugin contrib/plugins/libips.so,ips=1 -display none -serial mon:stdio
  **
  ERROR:../../tcg/tcg.c:3167:remove_label_use: code should not be reached
  Bail out! ERROR:../../tcg/tcg.c:3167:remove_label_use: code should not be reached
  fish: Job 1, './qemu-system-i386  -plugin con…' terminated by signal SIGABRT (Abort)


>
> On Mon, Sep 2, 2024, 16:08 Alex Bennée <[e-mail]> wrote:
>
>  Elisha Hollander <[e-mail]> writes:
>
>  > But for qemu_plugin_update_ns
>  >
>  > On Mon, Sep 2, 2024, 15:38 Elisha Hollander <[e-mail]> wrote:
>  >
>  >  Just checked with 9.0.2 it it still gives the error...
>  >
>  >  On Wed, Aug 28, 2024, 14:05 Alex Bennée <[e-mail]> wrote:
>  >
>  >  Elisha Hollander <[e-mail]> writes:
>  >
>  >  > Although it gives `undefined symbol: qemu_plugin_scoreboard_free`. But
>  >  > probably I messed something up...
>  >
>  >  Are you using an older QEMU? We should trigger an API warning if they
>  >  are mismatched but maybe thats not working.
>  >
>  >  >
>  >  > On Tue, Aug 27, 2024, 14:59 Elisha Hollander <[e-mail]> wrote:
>  >  >
>  >  >  Oh nice, I didn't know that
>  >  >
>  >  >  On Tue, Aug 27, 2024, 12:39 Alex Bennée <[e-mail]> wrote:
>  >  >
>  >  >  Elisha Hollander <[e-mail]> writes:
>  >  >
>  >  >  > Signed-off-by: Elisha Hollander <[e-mail]>
>  >  >
>  >  >  What is the use-case for this patch?
>  >  >
>  >  >  If you are simply looking to slow the emulated system down please have a
>  >  >  look at:
>  >  >
>  >  >    https://qemu.readthedocs.io/en/master/about/emulation.html#limit-instructions-per-second
>  >  >
>  >  >  which uses the plugin system to limit the run rate and sleep if its
>  >  >  running too fast. The longer term goal is to deprecate the icount clock
>  >  >  alignment feature from the core code and leave icount to just provide
>  >  >  the deterministic execution needed for record/replay and reverse
>  >  >  debugging.
>  >  >
>  >  >  > ---
>  >  >  >  accel/tcg/cpu-exec.c      | 4 +---
>  >  >  >  accel/tcg/icount-common.c | 4 ++--
>  >  >  >  2 files changed, 3 insertions(+), 5 deletions(-)
>  >  >  >
>  >  >  > diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
>  >  >  > index 8163295f34..4c2baf8ed4 100644
>  >  >  > --- a/accel/tcg/cpu-exec.c
>  >  >  > +++ b/accel/tcg/cpu-exec.c
>  >  >  > @@ -95,11 +95,10 @@ static void align_clocks(SyncClocks *sc, CPUState *cpu)
>  >  >  >  static void print_delay(const SyncClocks *sc)
>  >  >  >  {
>  >  >  >      static float threshold_delay;
>  >  >  > -    static int64_t last_realtime_clock;
>  >  >  >      static int nb_prints;
>  >  >  >  
>  >  >  >      if (icount_align_option &&
>  >  >  > -        sc->realtime_clock - last_realtime_clock >= MAX_DELAY_PRINT_RATE &&
>  >  >  > +        sc->diff_clk >= MAX_DELAY_PRINT_RATE &&
>  >  >  >          nb_prints < MAX_NB_PRINTS) {
>  >  >  >          if ((-sc->diff_clk / (float)1000000000LL > threshold_delay) ||
>  >  >  >              (-sc->diff_clk / (float)1000000000LL <
>  >  >  > @@ -109,7 +108,6 @@ static void print_delay(const SyncClocks *sc)
>  >  >  >                          threshold_delay - 1,
>  >  >  >                          threshold_delay);
>  >  >  >              nb_prints++;
>  >  >  > -            last_realtime_clock = sc->realtime_clock;
>  >  >  >          }
>  >  >  >      }
>  >  >  >  }
>  >  >  > diff --git a/accel/tcg/icount-common.c b/accel/tcg/icount-common.c
>  >  >  > index 8d3d3a7e9d..f07f8baf4d 100644
>  >  >  > --- a/accel/tcg/icount-common.c
>  >  >  > +++ b/accel/tcg/icount-common.c
>  >  >  > @@ -46,8 +46,8 @@
>  >  >  >   * is TCG-specific, and does not need to be built for other accels.
>  >  >  >   */
>  >  >  >  static bool icount_sleep = true;
>  >  >  > -/* Arbitrarily pick 1MIPS as the minimum allowable speed.  */
>  >  >  > -#define MAX_ICOUNT_SHIFT 10
>  >  >  > +/* Arbitrarily pick the minimum allowable speed.  */
>  >  >  > +#define MAX_ICOUNT_SHIFT 30
>  >  >  >  
>  >  >  >  /* Do not count executed instructions */
>  >  >  >  ICountMode use_icount = ICOUNT_DISABLED;
>  >  >
>  >  >  -- 
>  >  >  Alex Bennée
>  >  >  Virtualisation Tech Lead @ Linaro
>  >
>  >  -- 
>  >  Alex Bennée
>  >  Virtualisation Tech Lead @ Linaro
>
>  Can you give me your command line please?
>
>  -- 
>  Alex Bennée
>  Virtualisation Tech Lead @ Linaro

Alex Bennée Sept. 3, 2024, 11:13 a.m. UTC | #11

Alex Bennée <[e-mail]> writes:

> Elisha Hollander <[e-mail]> writes:
>
>> Sure!
>>
>> `build/qemu-system-i386 -plugin build/contrib/plugins/libips.so,ips=1 -display curses -bios bios.raw`
>>
>> Also, I just tested with 9.1.0-rc4 and it resulted with
>> `tcg.c:3167:remove_label_use: code should not be reached`
>
> I can replicate but it seems to be an edge case with ips=1, try a higher
> number, even ips=10 doesn't trigger the crash (but 10 instructions a
> second is very slow to do anything).
>
>   ./qemu-system-i386  -plugin contrib/plugins/libips.so,ips=1 -display none -serial mon:stdio
>   **
>   ERROR:../../tcg/tcg.c:3167:remove_label_use: code should not be reached
>   Bail out! ERROR:../../tcg/tcg.c:3167:remove_label_use: code should not be reached
>   fish: Job 1, './qemu-system-i386  -plugin con…' terminated by signal
>   SIGABRT (Abort)

Well this is fascinating and I suspect Richard needs to comment but:

  ./qemu-system-i386  -plugin contrib/plugins/libips.so,ips=9 -display none -serial mon:stdio -d in_asm,op,op_opt,out_asm -plugin contrib/plugins/libstoptrigger.so,icount=20 -D broken.log
  **
  ERROR:../../tcg/tcg.c:3167:remove_label_use: code should not be reached
  Bail out! ERROR:../../tcg/tcg.c:3167:remove_label_use: code should not be reached
  fish: Job 1, './qemu-system-i386  -plugin con…' terminated by signal SIGABRT (Abort)

and:

 ./qemu-system-i386  -plugin contrib/plugins/libips.so,ips=10 -display none -serial mon:stdio -d in_asm,op,op_opt,out_asm -plugin contrib/plugins/libstoptrigger.so,icount=20 -D working.log

But for the life of me I can't see the difference between the two sets
of input, the first crashes before the optimisation step:

PROLOGUE: [size=45]
0x7f2198000000:  55                       pushq    %rbp
0x7f2198000001:  53                       pushq    %rbx
0x7f2198000002:  41 54                    pushq    %r12
0x7f2198000004:  41 55                    pushq    %r13
0x7f2198000006:  41 56                    pushq    %r14
0x7f2198000008:  41 57                    pushq    %r15
0x7f219800000a:  48 8b ef                 movq     %rdi, %rbp
0x7f219800000d:  48 81 c4 78 fb ff ff     addq     $-0x488, %rsp
0x7f2198000014:  ff e6                    jmpq     *%rsi
0x7f2198000016:  33 c0                    xorl     %eax, %eax
0x7f2198000018:  48 81 c4 88 04 00 00     addq     $0x488, %rsp
0x7f219800001f:  c5 f8 77                 vzeroupper 
0x7f2198000022:  41 5f                    popq     %r15
0x7f2198000024:  41 5e                    popq     %r14
0x7f2198000026:  41 5d                    popq     %r13
0x7f2198000028:  41 5c                    popq     %r12
0x7f219800002a:  5b                       popq     %rbx
0x7f219800002b:  5d                       popq     %rbp
0x7f219800002c:  c3                       retq     

----------------
IN: 
0xfffffff0:  ea 5b e0 00 f0           ljmpw    $0xf000:$0xe05b

OP:
 ld_i32 loc9,env,$0xfffffffffffffff8
 brcond_i32 loc9,$0x0,lt,$L0
 ld_i32 tmp18,env,$0xffffffffffffdaf8
 mul_i32 tmp18,tmp18,$0x18
 ext_i32_i64 tmp17,tmp18
 add_i64 tmp17,tmp17,$0x55912c381e38
 ld_i64 tmp21,tmp17,$0x0
 add_i64 tmp21,tmp21,$0x1
 st_i64 tmp21,tmp17,$0x0
 ld_i32 tmp18,env,$0xffffffffffffdaf8
 mul_i32 tmp18,tmp18,$0x18
 ext_i32_i64 tmp17,tmp18
 add_i64 tmp17,tmp17,$0x55912c381e38
 ld_i64 tmp21,tmp17,$0x0
 brcond_i64 tmp21,$0x0,ltu,$L1
 ld_i32 tmp18,env,$0xffffffffffffdaf8
 call plugin(0x7f21efe82550),$0x1,$0,tmp18,$0x0
 set_label $L1
 st8_i32 $0x1,env,$0xfffffffffffffffc

 ---- 0000000000000ff0 0000000000000000
 ld_i32 tmp18,env,$0xffffffffffffdaf8
 shl_i32 tmp18,tmp18,$0x3
 ext_i32_i64 tmp17,tmp18
 add_i64 tmp17,tmp17,$0x55912c3803b0
 ld_i64 tmp21,tmp17,$0x0
 add_i64 tmp21,tmp21,$0x1
 st_i64 tmp21,tmp17,$0x0
 ld_i32 tmp18,env,$0xffffffffffffdaf8
 shl_i32 tmp18,tmp18,$0x3
 ext_i32_i64 tmp17,tmp18
 add_i64 tmp17,tmp17,$0x55912c3803b0
 ld_i64 tmp21,tmp17,$0x0
 brcond_i64 tmp21,$0x15,ne,$L2
 ld_i32 tmp18,env,$0xffffffffffffdaf8
 call plugin(0x7f21efe7d420),$0x1,$0,tmp18,$0xfffffff0
 set_label $L2
 mov_i32 loc0,$0xe05b
 mov_i32 loc1,$0xf000
 ext16u_i32 loc13,loc1
 st_i32 loc13,env,$0x54
 shl_i32 cs_base,loc13,$0x4
 mov_i32 eip,loc0
 call lookup_tb_ptr,$0x6,$1,tmp15,env
 goto_ptr tmp15
 set_label $L0
 exit_tb $0x7f2198000043

 BOOM!!!

and the working case:

PROLOGUE: [size=45]
0x7f110c000000:  55                       pushq    %rbp
0x7f110c000001:  53                       pushq    %rbx
0x7f110c000002:  41 54                    pushq    %r12
0x7f110c000004:  41 55                    pushq    %r13
0x7f110c000006:  41 56                    pushq    %r14
0x7f110c000008:  41 57                    pushq    %r15
0x7f110c00000a:  48 8b ef                 movq     %rdi, %rbp
0x7f110c00000d:  48 81 c4 78 fb ff ff     addq     $-0x488, %rsp
0x7f110c000014:  ff e6                    jmpq     *%rsi
0x7f110c000016:  33 c0                    xorl     %eax, %eax
0x7f110c000018:  48 81 c4 88 04 00 00     addq     $0x488, %rsp
0x7f110c00001f:  c5 f8 77                 vzeroupper 
0x7f110c000022:  41 5f                    popq     %r15
0x7f110c000024:  41 5e                    popq     %r14
0x7f110c000026:  41 5d                    popq     %r13
0x7f110c000028:  41 5c                    popq     %r12
0x7f110c00002a:  5b                       popq     %rbx
0x7f110c00002b:  5d                       popq     %rbp
0x7f110c00002c:  c3                       retq     

----------------
IN: 
0xfffffff0:  ea 5b e0 00 f0           ljmpw    $0xf000:$0xe05b

OP:
 ld_i32 loc9,env,$0xfffffffffffffff8
 brcond_i32 loc9,$0x0,lt,$L0
 ld_i32 tmp18,env,$0xffffffffffffdaf8
 mul_i32 tmp18,tmp18,$0x18
 ext_i32_i64 tmp17,tmp18
 add_i64 tmp17,tmp17,$0x55c2cb346e38
 ld_i64 tmp21,tmp17,$0x0
 add_i64 tmp21,tmp21,$0x1
 st_i64 tmp21,tmp17,$0x0
 ld_i32 tmp18,env,$0xffffffffffffdaf8
 mul_i32 tmp18,tmp18,$0x18
 ext_i32_i64 tmp17,tmp18
 add_i64 tmp17,tmp17,$0x55c2cb346e38
 ld_i64 tmp21,tmp17,$0x0
 brcond_i64 tmp21,$0x1,ltu,$L1
 ld_i32 tmp18,env,$0xffffffffffffdaf8
 call plugin(0x7f11633ab550),$0x1,$0,tmp18,$0x0
 set_label $L1
 st8_i32 $0x1,env,$0xfffffffffffffffc

 ---- 0000000000000ff0 0000000000000000
 ld_i32 tmp18,env,$0xffffffffffffdaf8
 shl_i32 tmp18,tmp18,$0x3
 ext_i32_i64 tmp17,tmp18
 add_i64 tmp17,tmp17,$0x55c2cb3453b0
 ld_i64 tmp21,tmp17,$0x0
 add_i64 tmp21,tmp21,$0x1
 st_i64 tmp21,tmp17,$0x0
 ld_i32 tmp18,env,$0xffffffffffffdaf8
 shl_i32 tmp18,tmp18,$0x3
 ext_i32_i64 tmp17,tmp18
 add_i64 tmp17,tmp17,$0x55c2cb3453b0
 ld_i64 tmp21,tmp17,$0x0
 brcond_i64 tmp21,$0x15,ne,$L2
 ld_i32 tmp18,env,$0xffffffffffffdaf8
 call plugin(0x7f11633a6420),$0x1,$0,tmp18,$0xfffffff0
 set_label $L2
 mov_i32 loc0,$0xe05b
 mov_i32 loc1,$0xf000
 ext16u_i32 loc13,loc1
 st_i32 loc13,env,$0x54
 shl_i32 cs_base,loc13,$0x4
 mov_i32 eip,loc0
 call lookup_tb_ptr,$0x6,$1,tmp15,env
 goto_ptr tmp15
 set_label $L0
 exit_tb $0x7f110c000043

OP after optimization and liveness analysis:
 ld_i32 tmp9,env,$0xfffffffffffffff8      pref=0xffff
 brcond_i32 tmp9,$0x0,lt,$L0              dead: 0 1
 ld_i32 tmp18,env,$0xffffffffffffdaf8     pref=0xffff
 mul_i32 tmp18,tmp18,$0x18                dead: 1  pref=0xffff
 ext_i32_i64 tmp17,tmp18                  dead: 1  pref=0xffff
 add_i64 tmp17,tmp17,$0x55c2cb346e38      dead: 1  pref=0xffff
 ld_i64 tmp21,tmp17,$0x0                  pref=0xffff
 add_i64 tmp21,tmp21,$0x1                 dead: 1  pref=0xffff
 st_i64 tmp21,tmp17,$0x0                  dead: 0 1
 ld_i32 tmp18,env,$0xffffffffffffdaf8     pref=0xffff
 mul_i32 tmp18,tmp18,$0x18                dead: 1 2  pref=0xffff
 ext_i32_i64 tmp17,tmp18                  dead: 1  pref=0xffff
 add_i64 tmp17,tmp17,$0x55c2cb346e38      dead: 1 2  pref=0xffff
 ld_i64 tmp21,tmp17,$0x0                  dead: 1  pref=0xffff
 brcond_i64 tmp21,$0x1,ltu,$L1            dead: 0 1
 ld_i32 tmp18,env,$0xffffffffffffdaf8     dead: 1  pref=0x80
 call plugin(0x7f11633ab550),$0x1,$0,tmp18,$0x0  dead: 0 1
 set_label $L1                          
 st8_i32 $0x1,env,$0xfffffffffffffffc     dead: 0

 ---- 0000000000000ff0 0000000000000000
 ld_i32 tmp18,env,$0xffffffffffffdaf8     pref=0xffff
 shl_i32 tmp18,tmp18,$0x3                 dead: 1  pref=0xffff
 ext_i32_i64 tmp17,tmp18                  dead: 1  pref=0xffff
 add_i64 tmp17,tmp17,$0x55c2cb3453b0      dead: 1  pref=0xffff
 ld_i64 tmp21,tmp17,$0x0                  pref=0xffff
 add_i64 tmp21,tmp21,$0x1                 dead: 1 2  pref=0xffff
 st_i64 tmp21,tmp17,$0x0                  dead: 0 1
 ld_i32 tmp18,env,$0xffffffffffffdaf8     pref=0xffff
 shl_i32 tmp18,tmp18,$0x3                 dead: 1 2  pref=0xffff
 ext_i32_i64 tmp17,tmp18                  dead: 1  pref=0xffff
 add_i64 tmp17,tmp17,$0x55c2cb3453b0      dead: 1 2  pref=0xffff
 ld_i64 tmp21,tmp17,$0x0                  dead: 1  pref=0xffff
 brcond_i64 tmp21,$0x15,ne,$L2            dead: 0 1
 ld_i32 tmp18,env,$0xffffffffffffdaf8     dead: 1  pref=0x80
 call plugin(0x7f11633a6420),$0x1,$0,tmp18,$0xfffffff0  dead: 0 1
 set_label $L2                          
 st_i32 $0xf000,env,$0x54                 dead: 0
 mov_i32 cs_base,$0xf0000                 sync: 0  dead: 0 1  pref=0xffff
 mov_i32 eip,$0xe05b                      sync: 0  dead: 0 1  pref=0xffff
 call lookup_tb_ptr,$0x6,$1,tmp15,env     dead: 1  pref=none
 goto_ptr tmp15                           dead: 0
 set_label $L0                          
 exit_tb $0x7f110c000043                

OUT: [size=224]
  -- guest addr 0x0000000000000ff0 + tb prologue
0x7f110c000100:  8b 5d f8                 movl     -8(%rbp), %ebx
0x7f110c000103:  85 db                    testl    %ebx, %ebx
0x7f110c000105:  0f 8c bd 00 00 00        jl       0x7f110c0001c8
0x7f110c00010b:  8b 9d f8 da ff ff        movl     -0x2508(%rbp), %ebx
0x7f110c000111:  6b db 18                 imull    $0x18, %ebx, %ebx
0x7f110c000114:  48 63 db                 movslq   %ebx, %rbx
0x7f110c000117:  49 bc 38 6e 34 cb c2 55  movabsq  $0x55c2cb346e38, %r12
0x7f110c00011f:  00 00
0x7f110c000121:  49 03 dc                 addq     %r12, %rbx
0x7f110c000124:  4c 8b 2b                 movq     (%rbx), %r13
0x7f110c000127:  49 ff c5                 incq     %r13
0x7f110c00012a:  4c 89 2b                 movq     %r13, (%rbx)
0x7f110c00012d:  8b 9d f8 da ff ff        movl     -0x2508(%rbp), %ebx
0x7f110c000133:  6b db 18                 imull    $0x18, %ebx, %ebx
0x7f110c000136:  48 63 db                 movslq   %ebx, %rbx
0x7f110c000139:  49 03 dc                 addq     %r12, %rbx
0x7f110c00013c:  48 8b 1b                 movq     (%rbx), %rbx
0x7f110c00013f:  48 83 fb 01              cmpq     $1, %rbx
0x7f110c000143:  0f 82 0d 00 00 00        jb       0x7f110c000156
0x7f110c000149:  8b bd f8 da ff ff        movl     -0x2508(%rbp), %edi
0x7f110c00014f:  33 f6                    xorl     %esi, %esi
0x7f110c000151:  e8 fa b3 3a 57           callq    0x7f11633ab550
0x7f110c000156:  c6 45 fc 01              movb     $1, -4(%rbp)
0x7f110c00015a:  8b 9d f8 da ff ff        movl     -0x2508(%rbp), %ebx
0x7f110c000160:  c1 e3 03                 shll     $3, %ebx
0x7f110c000163:  48 63 db                 movslq   %ebx, %rbx
0x7f110c000166:  49 bc b0 53 34 cb c2 55  movabsq  $0x55c2cb3453b0, %r12
0x7f110c00016e:  00 00
0x7f110c000170:  49 03 dc                 addq     %r12, %rbx
0x7f110c000173:  4c 8b 2b                 movq     (%rbx), %r13
0x7f110c000176:  49 ff c5                 incq     %r13
0x7f110c000179:  4c 89 2b                 movq     %r13, (%rbx)
0x7f110c00017c:  8b 9d f8 da ff ff        movl     -0x2508(%rbp), %ebx
0x7f110c000182:  c1 e3 03                 shll     $3, %ebx
0x7f110c000185:  48 63 db                 movslq   %ebx, %rbx
0x7f110c000188:  49 03 dc                 addq     %r12, %rbx
0x7f110c00018b:  48 8b 1b                 movq     (%rbx), %rbx
0x7f110c00018e:  48 83 fb 15              cmpq     $0x15, %rbx
0x7f110c000192:  0f 85 10 00 00 00        jne      0x7f110c0001a8
0x7f110c000198:  8b bd f8 da ff ff        movl     -0x2508(%rbp), %edi
0x7f110c00019e:  be f0 ff ff ff           movl     $0xfffffff0, %esi
0x7f110c0001a3:  e8 78 62 3a 57           callq    0x7f11633a6420
0x7f110c0001a8:  c7 45 54 00 f0 00 00     movl     $0xf000, 0x54(%rbp)
0x7f110c0001af:  c7 45 58 00 00 0f 00     movl     $0xf0000, 0x58(%rbp)
0x7f110c0001b6:  c7 45 20 5b e0 00 00     movl     $0xe05b, 0x20(%rbp)
0x7f110c0001bd:  48 8b fd                 movq     %rbp, %rdi
0x7f110c0001c0:  ff 15 12 00 00 00        callq    *0x12(%rip)
0x7f110c0001c6:  ff e0                    jmpq     *%rax
0x7f110c0001c8:  48 8d 05 74 fe ff ff     leaq     -0x18c(%rip), %rax
0x7f110c0001cf:  e9 44 fe ff ff           jmp      0x7f110c000018
  -- tb slow paths + alignment
0x7f110c0001d4:  90                       nop      
0x7f110c0001d5:  90                       nop      
0x7f110c0001d6:  90                       nop      
0x7f110c0001d7:  90                       nop      
  data: [size=8]
0x7f110c0001d8:  .quad  0x000055c2ba75abc0

before continuing to execute.

>
>
>>
>> On Mon, Sep 2, 2024, 16:08 Alex Bennée <[e-mail]> wrote:
>>
>>  Elisha Hollander <[e-mail]> writes:
>>
>>  > But for qemu_plugin_update_ns
>>  >
>>  > On Mon, Sep 2, 2024, 15:38 Elisha Hollander <[e-mail]> wrote:
>>  >
>>  >  Just checked with 9.0.2 it it still gives the error...
>>  >
>>  >  On Wed, Aug 28, 2024, 14:05 Alex Bennée <[e-mail]> wrote:
>>  >
>>  >  Elisha Hollander <[e-mail]> writes:
>>  >
>>  >  > Although it gives `undefined symbol: qemu_plugin_scoreboard_free`. But
>>  >  > probably I messed something up...
>>  >
>>  >  Are you using an older QEMU? We should trigger an API warning if they
>>  >  are mismatched but maybe thats not working.
>>  >
>>  >  >
>>  >  > On Tue, Aug 27, 2024, 14:59 Elisha Hollander <[e-mail]> wrote:
>>  >  >
>>  >  >  Oh nice, I didn't know that
>>  >  >
>>  >  >  On Tue, Aug 27, 2024, 12:39 Alex Bennée <[e-mail]> wrote:
>>  >  >
>>  >  >  Elisha Hollander <[e-mail]> writes:
>>  >  >
>>  >  >  > Signed-off-by: Elisha Hollander <[e-mail]>
>>  >  >
>>  >  >  What is the use-case for this patch?
>>  >  >
>>  >  >  If you are simply looking to slow the emulated system down please have a
>>  >  >  look at:
>>  >  >
>>  >  >    https://qemu.readthedocs.io/en/master/about/emulation.html#limit-instructions-per-second
>>  >  >
>>  >  >  which uses the plugin system to limit the run rate and sleep if its
>>  >  >  running too fast. The longer term goal is to deprecate the icount clock
>>  >  >  alignment feature from the core code and leave icount to just provide
>>  >  >  the deterministic execution needed for record/replay and reverse
>>  >  >  debugging.
>>  >  >
>>  >  >  > ---
>>  >  >  >  accel/tcg/cpu-exec.c      | 4 +---
>>  >  >  >  accel/tcg/icount-common.c | 4 ++--
>>  >  >  >  2 files changed, 3 insertions(+), 5 deletions(-)
>>  >  >  >
>>  >  >  > diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
>>  >  >  > index 8163295f34..4c2baf8ed4 100644
>>  >  >  > --- a/accel/tcg/cpu-exec.c
>>  >  >  > +++ b/accel/tcg/cpu-exec.c
>>  >  >  > @@ -95,11 +95,10 @@ static void align_clocks(SyncClocks *sc, CPUState *cpu)
>>  >  >  >  static void print_delay(const SyncClocks *sc)
>>  >  >  >  {
>>  >  >  >      static float threshold_delay;
>>  >  >  > -    static int64_t last_realtime_clock;
>>  >  >  >      static int nb_prints;
>>  >  >  >  
>>  >  >  >      if (icount_align_option &&
>>  >  >  > -        sc->realtime_clock - last_realtime_clock >= MAX_DELAY_PRINT_RATE &&
>>  >  >  > +        sc->diff_clk >= MAX_DELAY_PRINT_RATE &&
>>  >  >  >          nb_prints < MAX_NB_PRINTS) {
>>  >  >  >          if ((-sc->diff_clk / (float)1000000000LL > threshold_delay) ||
>>  >  >  >              (-sc->diff_clk / (float)1000000000LL <
>>  >  >  > @@ -109,7 +108,6 @@ static void print_delay(const SyncClocks *sc)
>>  >  >  >                          threshold_delay - 1,
>>  >  >  >                          threshold_delay);
>>  >  >  >              nb_prints++;
>>  >  >  > -            last_realtime_clock = sc->realtime_clock;
>>  >  >  >          }
>>  >  >  >      }
>>  >  >  >  }
>>  >  >  > diff --git a/accel/tcg/icount-common.c b/accel/tcg/icount-common.c
>>  >  >  > index 8d3d3a7e9d..f07f8baf4d 100644
>>  >  >  > --- a/accel/tcg/icount-common.c
>>  >  >  > +++ b/accel/tcg/icount-common.c
>>  >  >  > @@ -46,8 +46,8 @@
>>  >  >  >   * is TCG-specific, and does not need to be built for other accels.
>>  >  >  >   */
>>  >  >  >  static bool icount_sleep = true;
>>  >  >  > -/* Arbitrarily pick 1MIPS as the minimum allowable speed.  */
>>  >  >  > -#define MAX_ICOUNT_SHIFT 10
>>  >  >  > +/* Arbitrarily pick the minimum allowable speed.  */
>>  >  >  > +#define MAX_ICOUNT_SHIFT 30
>>  >  >  >  
>>  >  >  >  /* Do not count executed instructions */
>>  >  >  >  ICountMode use_icount = ICOUNT_DISABLED;
>>  >  >
>>  >  >  -- 
>>  >  >  Alex Bennée
>>  >  >  Virtualisation Tech Lead @ Linaro
>>  >
>>  >  -- 
>>  >  Alex Bennée
>>  >  Virtualisation Tech Lead @ Linaro
>>
>>  Can you give me your command line please?
>>
>>  -- 
>>  Alex Bennée
>>  Virtualisation Tech Lead @ Linaro

Elisha Hollander Sept. 3, 2024, 3:28 p.m. UTC | #12

weird...

On Tue, Sep 3, 2024, 14:13 Alex Bennée <[e-mail]> wrote:

> Alex Bennée <[e-mail]> writes:
>
> > Elisha Hollander <[e-mail]> writes:
> >
> >> Sure!
> >>
> >> `build/qemu-system-i386 -plugin build/contrib/plugins/libips.so,ips=1
> -display curses -bios bios.raw`
> >>
> >> Also, I just tested with 9.1.0-rc4 and it resulted with
> >> `tcg.c:3167:remove_label_use: code should not be reached`
> >
> > I can replicate but it seems to be an edge case with ips=1, try a higher
> > number, even ips=10 doesn't trigger the crash (but 10 instructions a
> > second is very slow to do anything).
> >
> >   ./qemu-system-i386  -plugin contrib/plugins/libips.so,ips=1 -display
> none -serial mon:stdio
> >   **
> >   ERROR:../../tcg/tcg.c:3167:remove_label_use: code should not be reached
> >   Bail out! ERROR:../../tcg/tcg.c:3167:remove_label_use: code should not
> be reached
> >   fish: Job 1, './qemu-system-i386  -plugin con…' terminated by signal
> >   SIGABRT (Abort)
>
> Well this is fascinating and I suspect Richard needs to comment but:
>
>   ./qemu-system-i386  -plugin contrib/plugins/libips.so,ips=9 -display
> none -serial mon:stdio -d in_asm,op,op_opt,out_asm -plugin
> contrib/plugins/libstoptrigger.so,icount=20 -D broken.log
>   **
>   ERROR:../../tcg/tcg.c:3167:remove_label_use: code should not be reached
>   Bail out! ERROR:../../tcg/tcg.c:3167:remove_label_use: code should not
> be reached
>   fish: Job 1, './qemu-system-i386  -plugin con…' terminated by signal
> SIGABRT (Abort)
>
> and:
>
>  ./qemu-system-i386  -plugin contrib/plugins/libips.so,ips=10 -display
> none -serial mon:stdio -d in_asm,op,op_opt,out_asm -plugin
> contrib/plugins/libstoptrigger.so,icount=20 -D working.log
>
> But for the life of me I can't see the difference between the two sets
> of input, the first crashes before the optimisation step:
>
> PROLOGUE: [size=45]
> 0x7f2198000000:  55                       pushq    %rbp
> 0x7f2198000001:  53                       pushq    %rbx
> 0x7f2198000002:  41 54                    pushq    %r12
> 0x7f2198000004:  41 55                    pushq    %r13
> 0x7f2198000006:  41 56                    pushq    %r14
> 0x7f2198000008:  41 57                    pushq    %r15
> 0x7f219800000a:  48 8b ef                 movq     %rdi, %rbp
> 0x7f219800000d:  48 81 c4 78 fb ff ff     addq     $-0x488, %rsp
> 0x7f2198000014:  ff e6                    jmpq     *%rsi
> 0x7f2198000016:  33 c0                    xorl     %eax, %eax
> 0x7f2198000018:  48 81 c4 88 04 00 00     addq     $0x488, %rsp
> 0x7f219800001f:  c5 f8 77                 vzeroupper
> 0x7f2198000022:  41 5f                    popq     %r15
> 0x7f2198000024:  41 5e                    popq     %r14
> 0x7f2198000026:  41 5d                    popq     %r13
> 0x7f2198000028:  41 5c                    popq     %r12
> 0x7f219800002a:  5b                       popq     %rbx
> 0x7f219800002b:  5d                       popq     %rbp
> 0x7f219800002c:  c3                       retq
>
> ----------------
> IN:
> 0xfffffff0:  ea 5b e0 00 f0           ljmpw    $0xf000:$0xe05b
>
> OP:
>  ld_i32 loc9,env,$0xfffffffffffffff8
>  brcond_i32 loc9,$0x0,lt,$L0
>  ld_i32 tmp18,env,$0xffffffffffffdaf8
>  mul_i32 tmp18,tmp18,$0x18
>  ext_i32_i64 tmp17,tmp18
>  add_i64 tmp17,tmp17,$0x55912c381e38
>  ld_i64 tmp21,tmp17,$0x0
>  add_i64 tmp21,tmp21,$0x1
>  st_i64 tmp21,tmp17,$0x0
>  ld_i32 tmp18,env,$0xffffffffffffdaf8
>  mul_i32 tmp18,tmp18,$0x18
>  ext_i32_i64 tmp17,tmp18
>  add_i64 tmp17,tmp17,$0x55912c381e38
>  ld_i64 tmp21,tmp17,$0x0
>  brcond_i64 tmp21,$0x0,ltu,$L1
>  ld_i32 tmp18,env,$0xffffffffffffdaf8
>  call plugin(0x7f21efe82550),$0x1,$0,tmp18,$0x0
>  set_label $L1
>  st8_i32 $0x1,env,$0xfffffffffffffffc
>
>  ---- 0000000000000ff0 0000000000000000
>  ld_i32 tmp18,env,$0xffffffffffffdaf8
>  shl_i32 tmp18,tmp18,$0x3
>  ext_i32_i64 tmp17,tmp18
>  add_i64 tmp17,tmp17,$0x55912c3803b0
>  ld_i64 tmp21,tmp17,$0x0
>  add_i64 tmp21,tmp21,$0x1
>  st_i64 tmp21,tmp17,$0x0
>  ld_i32 tmp18,env,$0xffffffffffffdaf8
>  shl_i32 tmp18,tmp18,$0x3
>  ext_i32_i64 tmp17,tmp18
>  add_i64 tmp17,tmp17,$0x55912c3803b0
>  ld_i64 tmp21,tmp17,$0x0
>  brcond_i64 tmp21,$0x15,ne,$L2
>  ld_i32 tmp18,env,$0xffffffffffffdaf8
>  call plugin(0x7f21efe7d420),$0x1,$0,tmp18,$0xfffffff0
>  set_label $L2
>  mov_i32 loc0,$0xe05b
>  mov_i32 loc1,$0xf000
>  ext16u_i32 loc13,loc1
>  st_i32 loc13,env,$0x54
>  shl_i32 cs_base,loc13,$0x4
>  mov_i32 eip,loc0
>  call lookup_tb_ptr,$0x6,$1,tmp15,env
>  goto_ptr tmp15
>  set_label $L0
>  exit_tb $0x7f2198000043
>
>  BOOM!!!
>
> and the working case:
>
> PROLOGUE: [size=45]
> 0x7f110c000000:  55                       pushq    %rbp
> 0x7f110c000001:  53                       pushq    %rbx
> 0x7f110c000002:  41 54                    pushq    %r12
> 0x7f110c000004:  41 55                    pushq    %r13
> 0x7f110c000006:  41 56                    pushq    %r14
> 0x7f110c000008:  41 57                    pushq    %r15
> 0x7f110c00000a:  48 8b ef                 movq     %rdi, %rbp
> 0x7f110c00000d:  48 81 c4 78 fb ff ff     addq     $-0x488, %rsp
> 0x7f110c000014:  ff e6                    jmpq     *%rsi
> 0x7f110c000016:  33 c0                    xorl     %eax, %eax
> 0x7f110c000018:  48 81 c4 88 04 00 00     addq     $0x488, %rsp
> 0x7f110c00001f:  c5 f8 77                 vzeroupper
> 0x7f110c000022:  41 5f                    popq     %r15
> 0x7f110c000024:  41 5e                    popq     %r14
> 0x7f110c000026:  41 5d                    popq     %r13
> 0x7f110c000028:  41 5c                    popq     %r12
> 0x7f110c00002a:  5b                       popq     %rbx
> 0x7f110c00002b:  5d                       popq     %rbp
> 0x7f110c00002c:  c3                       retq
>
> ----------------
> IN:
> 0xfffffff0:  ea 5b e0 00 f0           ljmpw    $0xf000:$0xe05b
>
> OP:
>  ld_i32 loc9,env,$0xfffffffffffffff8
>  brcond_i32 loc9,$0x0,lt,$L0
>  ld_i32 tmp18,env,$0xffffffffffffdaf8
>  mul_i32 tmp18,tmp18,$0x18
>  ext_i32_i64 tmp17,tmp18
>  add_i64 tmp17,tmp17,$0x55c2cb346e38
>  ld_i64 tmp21,tmp17,$0x0
>  add_i64 tmp21,tmp21,$0x1
>  st_i64 tmp21,tmp17,$0x0
>  ld_i32 tmp18,env,$0xffffffffffffdaf8
>  mul_i32 tmp18,tmp18,$0x18
>  ext_i32_i64 tmp17,tmp18
>  add_i64 tmp17,tmp17,$0x55c2cb346e38
>  ld_i64 tmp21,tmp17,$0x0
>  brcond_i64 tmp21,$0x1,ltu,$L1
>  ld_i32 tmp18,env,$0xffffffffffffdaf8
>  call plugin(0x7f11633ab550),$0x1,$0,tmp18,$0x0
>  set_label $L1
>  st8_i32 $0x1,env,$0xfffffffffffffffc
>
>  ---- 0000000000000ff0 0000000000000000
>  ld_i32 tmp18,env,$0xffffffffffffdaf8
>  shl_i32 tmp18,tmp18,$0x3
>  ext_i32_i64 tmp17,tmp18
>  add_i64 tmp17,tmp17,$0x55c2cb3453b0
>  ld_i64 tmp21,tmp17,$0x0
>  add_i64 tmp21,tmp21,$0x1
>  st_i64 tmp21,tmp17,$0x0
>  ld_i32 tmp18,env,$0xffffffffffffdaf8
>  shl_i32 tmp18,tmp18,$0x3
>  ext_i32_i64 tmp17,tmp18
>  add_i64 tmp17,tmp17,$0x55c2cb3453b0
>  ld_i64 tmp21,tmp17,$0x0
>  brcond_i64 tmp21,$0x15,ne,$L2
>  ld_i32 tmp18,env,$0xffffffffffffdaf8
>  call plugin(0x7f11633a6420),$0x1,$0,tmp18,$0xfffffff0
>  set_label $L2
>  mov_i32 loc0,$0xe05b
>  mov_i32 loc1,$0xf000
>  ext16u_i32 loc13,loc1
>  st_i32 loc13,env,$0x54
>  shl_i32 cs_base,loc13,$0x4
>  mov_i32 eip,loc0
>  call lookup_tb_ptr,$0x6,$1,tmp15,env
>  goto_ptr tmp15
>  set_label $L0
>  exit_tb $0x7f110c000043
>
> OP after optimization and liveness analysis:
>  ld_i32 tmp9,env,$0xfffffffffffffff8      pref=0xffff
>  brcond_i32 tmp9,$0x0,lt,$L0              dead: 0 1
>  ld_i32 tmp18,env,$0xffffffffffffdaf8     pref=0xffff
>  mul_i32 tmp18,tmp18,$0x18                dead: 1  pref=0xffff
>  ext_i32_i64 tmp17,tmp18                  dead: 1  pref=0xffff
>  add_i64 tmp17,tmp17,$0x55c2cb346e38      dead: 1  pref=0xffff
>  ld_i64 tmp21,tmp17,$0x0                  pref=0xffff
>  add_i64 tmp21,tmp21,$0x1                 dead: 1  pref=0xffff
>  st_i64 tmp21,tmp17,$0x0                  dead: 0 1
>  ld_i32 tmp18,env,$0xffffffffffffdaf8     pref=0xffff
>  mul_i32 tmp18,tmp18,$0x18                dead: 1 2  pref=0xffff
>  ext_i32_i64 tmp17,tmp18                  dead: 1  pref=0xffff
>  add_i64 tmp17,tmp17,$0x55c2cb346e38      dead: 1 2  pref=0xffff
>  ld_i64 tmp21,tmp17,$0x0                  dead: 1  pref=0xffff
>  brcond_i64 tmp21,$0x1,ltu,$L1            dead: 0 1
>  ld_i32 tmp18,env,$0xffffffffffffdaf8     dead: 1  pref=0x80
>  call plugin(0x7f11633ab550),$0x1,$0,tmp18,$0x0  dead: 0 1
>  set_label $L1
>  st8_i32 $0x1,env,$0xfffffffffffffffc     dead: 0
>
>  ---- 0000000000000ff0 0000000000000000
>  ld_i32 tmp18,env,$0xffffffffffffdaf8     pref=0xffff
>  shl_i32 tmp18,tmp18,$0x3                 dead: 1  pref=0xffff
>  ext_i32_i64 tmp17,tmp18                  dead: 1  pref=0xffff
>  add_i64 tmp17,tmp17,$0x55c2cb3453b0      dead: 1  pref=0xffff
>  ld_i64 tmp21,tmp17,$0x0                  pref=0xffff
>  add_i64 tmp21,tmp21,$0x1                 dead: 1 2  pref=0xffff
>  st_i64 tmp21,tmp17,$0x0                  dead: 0 1
>  ld_i32 tmp18,env,$0xffffffffffffdaf8     pref=0xffff
>  shl_i32 tmp18,tmp18,$0x3                 dead: 1 2  pref=0xffff
>  ext_i32_i64 tmp17,tmp18                  dead: 1  pref=0xffff
>  add_i64 tmp17,tmp17,$0x55c2cb3453b0      dead: 1 2  pref=0xffff
>  ld_i64 tmp21,tmp17,$0x0                  dead: 1  pref=0xffff
>  brcond_i64 tmp21,$0x15,ne,$L2            dead: 0 1
>  ld_i32 tmp18,env,$0xffffffffffffdaf8     dead: 1  pref=0x80
>  call plugin(0x7f11633a6420),$0x1,$0,tmp18,$0xfffffff0  dead: 0 1
>  set_label $L2
>  st_i32 $0xf000,env,$0x54                 dead: 0
>  mov_i32 cs_base,$0xf0000                 sync: 0  dead: 0 1  pref=0xffff
>  mov_i32 eip,$0xe05b                      sync: 0  dead: 0 1  pref=0xffff
>  call lookup_tb_ptr,$0x6,$1,tmp15,env     dead: 1  pref=none
>  goto_ptr tmp15                           dead: 0
>  set_label $L0
>  exit_tb $0x7f110c000043
>
> OUT: [size=224]
>   -- guest addr 0x0000000000000ff0 + tb prologue
> 0x7f110c000100:  8b 5d f8                 movl     -8(%rbp), %ebx
> 0x7f110c000103:  85 db                    testl    %ebx, %ebx
> 0x7f110c000105:  0f 8c bd 00 00 00        jl       0x7f110c0001c8
> 0x7f110c00010b:  8b 9d f8 da ff ff        movl     -0x2508(%rbp), %ebx
> 0x7f110c000111:  6b db 18                 imull    $0x18, %ebx, %ebx
> 0x7f110c000114:  48 63 db                 movslq   %ebx, %rbx
> 0x7f110c000117:  49 bc 38 6e 34 cb c2 55  movabsq  $0x55c2cb346e38, %r12
> 0x7f110c00011f:  00 00
> 0x7f110c000121:  49 03 dc                 addq     %r12, %rbx
> 0x7f110c000124:  4c 8b 2b                 movq     (%rbx), %r13
> 0x7f110c000127:  49 ff c5                 incq     %r13
> 0x7f110c00012a:  4c 89 2b                 movq     %r13, (%rbx)
> 0x7f110c00012d:  8b 9d f8 da ff ff        movl     -0x2508(%rbp), %ebx
> 0x7f110c000133:  6b db 18                 imull    $0x18, %ebx, %ebx
> 0x7f110c000136:  48 63 db                 movslq   %ebx, %rbx
> 0x7f110c000139:  49 03 dc                 addq     %r12, %rbx
> 0x7f110c00013c:  48 8b 1b                 movq     (%rbx), %rbx
> 0x7f110c00013f:  48 83 fb 01              cmpq     $1, %rbx
> 0x7f110c000143:  0f 82 0d 00 00 00        jb       0x7f110c000156
> 0x7f110c000149:  8b bd f8 da ff ff        movl     -0x2508(%rbp), %edi
> 0x7f110c00014f:  33 f6                    xorl     %esi, %esi
> 0x7f110c000151:  e8 fa b3 3a 57           callq    0x7f11633ab550
> 0x7f110c000156:  c6 45 fc 01              movb     $1, -4(%rbp)
> 0x7f110c00015a:  8b 9d f8 da ff ff        movl     -0x2508(%rbp), %ebx
> 0x7f110c000160:  c1 e3 03                 shll     $3, %ebx
> 0x7f110c000163:  48 63 db                 movslq   %ebx, %rbx
> 0x7f110c000166:  49 bc b0 53 34 cb c2 55  movabsq  $0x55c2cb3453b0, %r12
> 0x7f110c00016e:  00 00
> 0x7f110c000170:  49 03 dc                 addq     %r12, %rbx
> 0x7f110c000173:  4c 8b 2b                 movq     (%rbx), %r13
> 0x7f110c000176:  49 ff c5                 incq     %r13
> 0x7f110c000179:  4c 89 2b                 movq     %r13, (%rbx)
> 0x7f110c00017c:  8b 9d f8 da ff ff        movl     -0x2508(%rbp), %ebx
> 0x7f110c000182:  c1 e3 03                 shll     $3, %ebx
> 0x7f110c000185:  48 63 db                 movslq   %ebx, %rbx
> 0x7f110c000188:  49 03 dc                 addq     %r12, %rbx
> 0x7f110c00018b:  48 8b 1b                 movq     (%rbx), %rbx
> 0x7f110c00018e:  48 83 fb 15              cmpq     $0x15, %rbx
> 0x7f110c000192:  0f 85 10 00 00 00        jne      0x7f110c0001a8
> 0x7f110c000198:  8b bd f8 da ff ff        movl     -0x2508(%rbp), %edi
> 0x7f110c00019e:  be f0 ff ff ff           movl     $0xfffffff0, %esi
> 0x7f110c0001a3:  e8 78 62 3a 57           callq    0x7f11633a6420
> 0x7f110c0001a8:  c7 45 54 00 f0 00 00     movl     $0xf000, 0x54(%rbp)
> 0x7f110c0001af:  c7 45 58 00 00 0f 00     movl     $0xf0000, 0x58(%rbp)
> 0x7f110c0001b6:  c7 45 20 5b e0 00 00     movl     $0xe05b, 0x20(%rbp)
> 0x7f110c0001bd:  48 8b fd                 movq     %rbp, %rdi
> 0x7f110c0001c0:  ff 15 12 00 00 00        callq    *0x12(%rip)
> 0x7f110c0001c6:  ff e0                    jmpq     *%rax
> 0x7f110c0001c8:  48 8d 05 74 fe ff ff     leaq     -0x18c(%rip), %rax
> 0x7f110c0001cf:  e9 44 fe ff ff           jmp      0x7f110c000018
>   -- tb slow paths + alignment
> 0x7f110c0001d4:  90                       nop
> 0x7f110c0001d5:  90                       nop
> 0x7f110c0001d6:  90                       nop
> 0x7f110c0001d7:  90                       nop
>   data: [size=8]
> 0x7f110c0001d8:  .quad  0x000055c2ba75abc0
>
> before continuing to execute.
>
> >
> >
> >>
> >> On Mon, Sep 2, 2024, 16:08 Alex Bennée <[e-mail]> wrote:
> >>
> >>  Elisha Hollander <[e-mail]> writes:
> >>
> >>  > But for qemu_plugin_update_ns
> >>  >
> >>  > On Mon, Sep 2, 2024, 15:38 Elisha Hollander <
> [e-mail]> wrote:
> >>  >
> >>  >  Just checked with 9.0.2 it it still gives the error...
> >>  >
> >>  >  On Wed, Aug 28, 2024, 14:05 Alex Bennée <[e-mail]>
> wrote:
> >>  >
> >>  >  Elisha Hollander <[e-mail]> writes:
> >>  >
> >>  >  > Although it gives `undefined symbol:
> qemu_plugin_scoreboard_free`. But
> >>  >  > probably I messed something up...
> >>  >
> >>  >  Are you using an older QEMU? We should trigger an API warning if
> they
> >>  >  are mismatched but maybe thats not working.
> >>  >
> >>  >  >
> >>  >  > On Tue, Aug 27, 2024, 14:59 Elisha Hollander <
> [e-mail]> wrote:
> >>  >  >
> >>  >  >  Oh nice, I didn't know that
> >>  >  >
> >>  >  >  On Tue, Aug 27, 2024, 12:39 Alex Bennée <[e-mail]>
> wrote:
> >>  >  >
> >>  >  >  Elisha Hollander <[e-mail]> writes:
> >>  >  >
> >>  >  >  > Signed-off-by: Elisha Hollander <[e-mail]>
> >>  >  >
> >>  >  >  What is the use-case for this patch?
> >>  >  >
> >>  >  >  If you are simply looking to slow the emulated system down
> please have a
> >>  >  >  look at:
> >>  >  >
> >>  >  >
> https://qemu.readthedocs.io/en/master/about/emulation.html#limit-instructions-per-second
> >>  >  >
> >>  >  >  which uses the plugin system to limit the run rate and sleep if
> its
> >>  >  >  running too fast. The longer term goal is to deprecate the
> icount clock
> >>  >  >  alignment feature from the core code and leave icount to just
> provide
> >>  >  >  the deterministic execution needed for record/replay and reverse
> >>  >  >  debugging.
> >>  >  >
> >>  >  >  > ---
> >>  >  >  >  accel/tcg/cpu-exec.c      | 4 +---
> >>  >  >  >  accel/tcg/icount-common.c | 4 ++--
> >>  >  >  >  2 files changed, 3 insertions(+), 5 deletions(-)
> >>  >  >  >
> >>  >  >  > diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> >>  >  >  > index 8163295f34..4c2baf8ed4 100644
> >>  >  >  > --- a/accel/tcg/cpu-exec.c
> >>  >  >  > +++ b/accel/tcg/cpu-exec.c
> >>  >  >  > @@ -95,11 +95,10 @@ static void align_clocks(SyncClocks *sc,
> CPUState *cpu)
> >>  >  >  >  static void print_delay(const SyncClocks *sc)
> >>  >  >  >  {
> >>  >  >  >      static float threshold_delay;
> >>  >  >  > -    static int64_t last_realtime_clock;
> >>  >  >  >      static int nb_prints;
> >>  >  >  >
> >>  >  >  >      if (icount_align_option &&
> >>  >  >  > -        sc->realtime_clock - last_realtime_clock >=
> MAX_DELAY_PRINT_RATE &&
> >>  >  >  > +        sc->diff_clk >= MAX_DELAY_PRINT_RATE &&
> >>  >  >  >          nb_prints < MAX_NB_PRINTS) {
> >>  >  >  >          if ((-sc->diff_clk / (float)1000000000LL >
> threshold_delay) ||
> >>  >  >  >              (-sc->diff_clk / (float)1000000000LL <
> >>  >  >  > @@ -109,7 +108,6 @@ static void print_delay(const SyncClocks
> *sc)
> >>  >  >  >                          threshold_delay - 1,
> >>  >  >  >                          threshold_delay);
> >>  >  >  >              nb_prints++;
> >>  >  >  > -            last_realtime_clock = sc->realtime_clock;
> >>  >  >  >          }
> >>  >  >  >      }
> >>  >  >  >  }
> >>  >  >  > diff --git a/accel/tcg/icount-common.c
> b/accel/tcg/icount-common.c
> >>  >  >  > index 8d3d3a7e9d..f07f8baf4d 100644
> >>  >  >  > --- a/accel/tcg/icount-common.c
> >>  >  >  > +++ b/accel/tcg/icount-common.c
> >>  >  >  > @@ -46,8 +46,8 @@
> >>  >  >  >   * is TCG-specific, and does not need to be built for other
> accels.
> >>  >  >  >   */
> >>  >  >  >  static bool icount_sleep = true;
> >>  >  >  > -/* Arbitrarily pick 1MIPS as the minimum allowable speed.  */
> >>  >  >  > -#define MAX_ICOUNT_SHIFT 10
> >>  >  >  > +/* Arbitrarily pick the minimum allowable speed.  */
> >>  >  >  > +#define MAX_ICOUNT_SHIFT 30
> >>  >  >  >
> >>  >  >  >  /* Do not count executed instructions */
> >>  >  >  >  ICountMode use_icount = ICOUNT_DISABLED;
> >>  >  >
> >>  >  >  --
> >>  >  >  Alex Bennée
> >>  >  >  Virtualisation Tech Lead @ Linaro
> >>  >
> >>  >  --
> >>  >  Alex Bennée
> >>  >  Virtualisation Tech Lead @ Linaro
> >>
> >>  Can you give me your command line please?
> >>
> >>  --
> >>  Alex Bennée
> >>  Virtualisation Tech Lead @ Linaro
>
> --
> Alex Bennée
> Virtualisation Tech Lead @ Linaro
>

Alex Bennée Sept. 13, 2024, 10:06 a.m. UTC | #13

Elisha Hollander <[e-mail]> writes:

> weird...

Richard has posted some patches:

  Message-ID: <[e-mail]>
  Date: Tue, 10 Sep 2024 14:23:49 -0700
  Subject: [PATCH 0/2] tcg: Fix branch/label link during plugin expansion
  From: Richard Henderson <[e-mail]>

which work for me. Can you try them out?

>
> On Tue, Sep 3, 2024, 14:13 Alex Bennée <[e-mail]> wrote:
>
>  Alex Bennée <[e-mail]> writes:
>
>  > Elisha Hollander <[e-mail]> writes:
>  >
>  >> Sure!
>  >>
>  >> `build/qemu-system-i386 -plugin build/contrib/plugins/libips.so,ips=1 -display curses -bios bios.raw`
>  >>
>  >> Also, I just tested with 9.1.0-rc4 and it resulted with
>  >> `tcg.c:3167:remove_label_use: code should not be reached`
>  >
>  > I can replicate but it seems to be an edge case with ips=1, try a higher
>  > number, even ips=10 doesn't trigger the crash (but 10 instructions a
>  > second is very slow to do anything).
>  >
>  >   ./qemu-system-i386  -plugin contrib/plugins/libips.so,ips=1 -display none -serial mon:stdio
>  >   **
>  >   ERROR:../../tcg/tcg.c:3167:remove_label_use: code should not be reached
>  >   Bail out! ERROR:../../tcg/tcg.c:3167:remove_label_use: code should not be reached
>  >   fish: Job 1, './qemu-system-i386  -plugin con…' terminated by signal
>  >   SIGABRT (Abort)
>
>  Well this is fascinating and I suspect Richard needs to comment but:
>
>    ./qemu-system-i386  -plugin contrib/plugins/libips.so,ips=9 -display none -serial mon:stdio -d in_asm,op,op_opt,out_asm
>  -plugin contrib/plugins/libstoptrigger.so,icount=20 -D broken.log
>    **
>    ERROR:../../tcg/tcg.c:3167:remove_label_use: code should not be reached
>    Bail out! ERROR:../../tcg/tcg.c:3167:remove_label_use: code should not be reached
>    fish: Job 1, './qemu-system-i386  -plugin con…' terminated by signal SIGABRT (Abort)
>
>  and:
>
>   ./qemu-system-i386  -plugin contrib/plugins/libips.so,ips=10 -display none -serial mon:stdio -d in_asm,op,op_opt,out_asm
>  -plugin contrib/plugins/libstoptrigger.so,icount=20 -D working.log
>
>  But for the life of me I can't see the difference between the two sets
>  of input, the first crashes before the optimisation step:
>
>  PROLOGUE: [size=45]
>  0x7f2198000000:  55                       pushq    %rbp
>  0x7f2198000001:  53                       pushq    %rbx
>  0x7f2198000002:  41 54                    pushq    %r12
>  0x7f2198000004:  41 55                    pushq    %r13
>  0x7f2198000006:  41 56                    pushq    %r14
>  0x7f2198000008:  41 57                    pushq    %r15
>  0x7f219800000a:  48 8b ef                 movq     %rdi, %rbp
>  0x7f219800000d:  48 81 c4 78 fb ff ff     addq     $-0x488, %rsp
>  0x7f2198000014:  ff e6                    jmpq     *%rsi
>  0x7f2198000016:  33 c0                    xorl     %eax, %eax
>  0x7f2198000018:  48 81 c4 88 04 00 00     addq     $0x488, %rsp
>  0x7f219800001f:  c5 f8 77                 vzeroupper 
>  0x7f2198000022:  41 5f                    popq     %r15
>  0x7f2198000024:  41 5e                    popq     %r14
>  0x7f2198000026:  41 5d                    popq     %r13
>  0x7f2198000028:  41 5c                    popq     %r12
>  0x7f219800002a:  5b                       popq     %rbx
>  0x7f219800002b:  5d                       popq     %rbp
>  0x7f219800002c:  c3                       retq     
>
>  ----------------
>  IN: 
>  0xfffffff0:  ea 5b e0 00 f0           ljmpw    $0xf000:$0xe05b
>
>  OP:
>   ld_i32 loc9,env,$0xfffffffffffffff8
>   brcond_i32 loc9,$0x0,lt,$L0
>   ld_i32 tmp18,env,$0xffffffffffffdaf8
>   mul_i32 tmp18,tmp18,$0x18
>   ext_i32_i64 tmp17,tmp18
>   add_i64 tmp17,tmp17,$0x55912c381e38
>   ld_i64 tmp21,tmp17,$0x0
>   add_i64 tmp21,tmp21,$0x1
>   st_i64 tmp21,tmp17,$0x0
>   ld_i32 tmp18,env,$0xffffffffffffdaf8
>   mul_i32 tmp18,tmp18,$0x18
>   ext_i32_i64 tmp17,tmp18
>   add_i64 tmp17,tmp17,$0x55912c381e38
>   ld_i64 tmp21,tmp17,$0x0
>   brcond_i64 tmp21,$0x0,ltu,$L1
>   ld_i32 tmp18,env,$0xffffffffffffdaf8
>   call plugin(0x7f21efe82550),$0x1,$0,tmp18,$0x0
>   set_label $L1
>   st8_i32 $0x1,env,$0xfffffffffffffffc
>
>   ---- 0000000000000ff0 0000000000000000
>   ld_i32 tmp18,env,$0xffffffffffffdaf8
>   shl_i32 tmp18,tmp18,$0x3
>   ext_i32_i64 tmp17,tmp18
>   add_i64 tmp17,tmp17,$0x55912c3803b0
>   ld_i64 tmp21,tmp17,$0x0
>   add_i64 tmp21,tmp21,$0x1
>   st_i64 tmp21,tmp17,$0x0
>   ld_i32 tmp18,env,$0xffffffffffffdaf8
>   shl_i32 tmp18,tmp18,$0x3
>   ext_i32_i64 tmp17,tmp18
>   add_i64 tmp17,tmp17,$0x55912c3803b0
>   ld_i64 tmp21,tmp17,$0x0
>   brcond_i64 tmp21,$0x15,ne,$L2
>   ld_i32 tmp18,env,$0xffffffffffffdaf8
>   call plugin(0x7f21efe7d420),$0x1,$0,tmp18,$0xfffffff0
>   set_label $L2
>   mov_i32 loc0,$0xe05b
>   mov_i32 loc1,$0xf000
>   ext16u_i32 loc13,loc1
>   st_i32 loc13,env,$0x54
>   shl_i32 cs_base,loc13,$0x4
>   mov_i32 eip,loc0
>   call lookup_tb_ptr,$0x6,$1,tmp15,env
>   goto_ptr tmp15
>   set_label $L0
>   exit_tb $0x7f2198000043
>
>   BOOM!!!
>
>  and the working case:
>
>  PROLOGUE: [size=45]
>  0x7f110c000000:  55                       pushq    %rbp
>  0x7f110c000001:  53                       pushq    %rbx
>  0x7f110c000002:  41 54                    pushq    %r12
>  0x7f110c000004:  41 55                    pushq    %r13
>  0x7f110c000006:  41 56                    pushq    %r14
>  0x7f110c000008:  41 57                    pushq    %r15
>  0x7f110c00000a:  48 8b ef                 movq     %rdi, %rbp
>  0x7f110c00000d:  48 81 c4 78 fb ff ff     addq     $-0x488, %rsp
>  0x7f110c000014:  ff e6                    jmpq     *%rsi
>  0x7f110c000016:  33 c0                    xorl     %eax, %eax
>  0x7f110c000018:  48 81 c4 88 04 00 00     addq     $0x488, %rsp
>  0x7f110c00001f:  c5 f8 77                 vzeroupper 
>  0x7f110c000022:  41 5f                    popq     %r15
>  0x7f110c000024:  41 5e                    popq     %r14
>  0x7f110c000026:  41 5d                    popq     %r13
>  0x7f110c000028:  41 5c                    popq     %r12
>  0x7f110c00002a:  5b                       popq     %rbx
>  0x7f110c00002b:  5d                       popq     %rbp
>  0x7f110c00002c:  c3                       retq     
>
>  ----------------
>  IN: 
>  0xfffffff0:  ea 5b e0 00 f0           ljmpw    $0xf000:$0xe05b
>
>  OP:
>   ld_i32 loc9,env,$0xfffffffffffffff8
>   brcond_i32 loc9,$0x0,lt,$L0
>   ld_i32 tmp18,env,$0xffffffffffffdaf8
>   mul_i32 tmp18,tmp18,$0x18
>   ext_i32_i64 tmp17,tmp18
>   add_i64 tmp17,tmp17,$0x55c2cb346e38
>   ld_i64 tmp21,tmp17,$0x0
>   add_i64 tmp21,tmp21,$0x1
>   st_i64 tmp21,tmp17,$0x0
>   ld_i32 tmp18,env,$0xffffffffffffdaf8
>   mul_i32 tmp18,tmp18,$0x18
>   ext_i32_i64 tmp17,tmp18
>   add_i64 tmp17,tmp17,$0x55c2cb346e38
>   ld_i64 tmp21,tmp17,$0x0
>   brcond_i64 tmp21,$0x1,ltu,$L1
>   ld_i32 tmp18,env,$0xffffffffffffdaf8
>   call plugin(0x7f11633ab550),$0x1,$0,tmp18,$0x0
>   set_label $L1
>   st8_i32 $0x1,env,$0xfffffffffffffffc
>
>   ---- 0000000000000ff0 0000000000000000
>   ld_i32 tmp18,env,$0xffffffffffffdaf8
>   shl_i32 tmp18,tmp18,$0x3
>   ext_i32_i64 tmp17,tmp18
>   add_i64 tmp17,tmp17,$0x55c2cb3453b0
>   ld_i64 tmp21,tmp17,$0x0
>   add_i64 tmp21,tmp21,$0x1
>   st_i64 tmp21,tmp17,$0x0
>   ld_i32 tmp18,env,$0xffffffffffffdaf8
>   shl_i32 tmp18,tmp18,$0x3
>   ext_i32_i64 tmp17,tmp18
>   add_i64 tmp17,tmp17,$0x55c2cb3453b0
>   ld_i64 tmp21,tmp17,$0x0
>   brcond_i64 tmp21,$0x15,ne,$L2
>   ld_i32 tmp18,env,$0xffffffffffffdaf8
>   call plugin(0x7f11633a6420),$0x1,$0,tmp18,$0xfffffff0
>   set_label $L2
>   mov_i32 loc0,$0xe05b
>   mov_i32 loc1,$0xf000
>   ext16u_i32 loc13,loc1
>   st_i32 loc13,env,$0x54
>   shl_i32 cs_base,loc13,$0x4
>   mov_i32 eip,loc0
>   call lookup_tb_ptr,$0x6,$1,tmp15,env
>   goto_ptr tmp15
>   set_label $L0
>   exit_tb $0x7f110c000043
>
>  OP after optimization and liveness analysis:
>   ld_i32 tmp9,env,$0xfffffffffffffff8      pref=0xffff
>   brcond_i32 tmp9,$0x0,lt,$L0              dead: 0 1
>   ld_i32 tmp18,env,$0xffffffffffffdaf8     pref=0xffff
>   mul_i32 tmp18,tmp18,$0x18                dead: 1  pref=0xffff
>   ext_i32_i64 tmp17,tmp18                  dead: 1  pref=0xffff
>   add_i64 tmp17,tmp17,$0x55c2cb346e38      dead: 1  pref=0xffff
>   ld_i64 tmp21,tmp17,$0x0                  pref=0xffff
>   add_i64 tmp21,tmp21,$0x1                 dead: 1  pref=0xffff
>   st_i64 tmp21,tmp17,$0x0                  dead: 0 1
>   ld_i32 tmp18,env,$0xffffffffffffdaf8     pref=0xffff
>   mul_i32 tmp18,tmp18,$0x18                dead: 1 2  pref=0xffff
>   ext_i32_i64 tmp17,tmp18                  dead: 1  pref=0xffff
>   add_i64 tmp17,tmp17,$0x55c2cb346e38      dead: 1 2  pref=0xffff
>   ld_i64 tmp21,tmp17,$0x0                  dead: 1  pref=0xffff
>   brcond_i64 tmp21,$0x1,ltu,$L1            dead: 0 1
>   ld_i32 tmp18,env,$0xffffffffffffdaf8     dead: 1  pref=0x80
>   call plugin(0x7f11633ab550),$0x1,$0,tmp18,$0x0  dead: 0 1
>   set_label $L1                          
>   st8_i32 $0x1,env,$0xfffffffffffffffc     dead: 0
>
>   ---- 0000000000000ff0 0000000000000000
>   ld_i32 tmp18,env,$0xffffffffffffdaf8     pref=0xffff
>   shl_i32 tmp18,tmp18,$0x3                 dead: 1  pref=0xffff
>   ext_i32_i64 tmp17,tmp18                  dead: 1  pref=0xffff
>   add_i64 tmp17,tmp17,$0x55c2cb3453b0      dead: 1  pref=0xffff
>   ld_i64 tmp21,tmp17,$0x0                  pref=0xffff
>   add_i64 tmp21,tmp21,$0x1                 dead: 1 2  pref=0xffff
>   st_i64 tmp21,tmp17,$0x0                  dead: 0 1
>   ld_i32 tmp18,env,$0xffffffffffffdaf8     pref=0xffff
>   shl_i32 tmp18,tmp18,$0x3                 dead: 1 2  pref=0xffff
>   ext_i32_i64 tmp17,tmp18                  dead: 1  pref=0xffff
>   add_i64 tmp17,tmp17,$0x55c2cb3453b0      dead: 1 2  pref=0xffff
>   ld_i64 tmp21,tmp17,$0x0                  dead: 1  pref=0xffff
>   brcond_i64 tmp21,$0x15,ne,$L2            dead: 0 1
>   ld_i32 tmp18,env,$0xffffffffffffdaf8     dead: 1  pref=0x80
>   call plugin(0x7f11633a6420),$0x1,$0,tmp18,$0xfffffff0  dead: 0 1
>   set_label $L2                          
>   st_i32 $0xf000,env,$0x54                 dead: 0
>   mov_i32 cs_base,$0xf0000                 sync: 0  dead: 0 1  pref=0xffff
>   mov_i32 eip,$0xe05b                      sync: 0  dead: 0 1  pref=0xffff
>   call lookup_tb_ptr,$0x6,$1,tmp15,env     dead: 1  pref=none
>   goto_ptr tmp15                           dead: 0
>   set_label $L0                          
>   exit_tb $0x7f110c000043                
>
>  OUT: [size=224]
>    -- guest addr 0x0000000000000ff0 + tb prologue
>  0x7f110c000100:  8b 5d f8                 movl     -8(%rbp), %ebx
>  0x7f110c000103:  85 db                    testl    %ebx, %ebx
>  0x7f110c000105:  0f 8c bd 00 00 00        jl       0x7f110c0001c8
>  0x7f110c00010b:  8b 9d f8 da ff ff        movl     -0x2508(%rbp), %ebx
>  0x7f110c000111:  6b db 18                 imull    $0x18, %ebx, %ebx
>  0x7f110c000114:  48 63 db                 movslq   %ebx, %rbx
>  0x7f110c000117:  49 bc 38 6e 34 cb c2 55  movabsq  $0x55c2cb346e38, %r12
>  0x7f110c00011f:  00 00
>  0x7f110c000121:  49 03 dc                 addq     %r12, %rbx
>  0x7f110c000124:  4c 8b 2b                 movq     (%rbx), %r13
>  0x7f110c000127:  49 ff c5                 incq     %r13
>  0x7f110c00012a:  4c 89 2b                 movq     %r13, (%rbx)
>  0x7f110c00012d:  8b 9d f8 da ff ff        movl     -0x2508(%rbp), %ebx
>  0x7f110c000133:  6b db 18                 imull    $0x18, %ebx, %ebx
>  0x7f110c000136:  48 63 db                 movslq   %ebx, %rbx
>  0x7f110c000139:  49 03 dc                 addq     %r12, %rbx
>  0x7f110c00013c:  48 8b 1b                 movq     (%rbx), %rbx
>  0x7f110c00013f:  48 83 fb 01              cmpq     $1, %rbx
>  0x7f110c000143:  0f 82 0d 00 00 00        jb       0x7f110c000156
>  0x7f110c000149:  8b bd f8 da ff ff        movl     -0x2508(%rbp), %edi
>  0x7f110c00014f:  33 f6                    xorl     %esi, %esi
>  0x7f110c000151:  e8 fa b3 3a 57           callq    0x7f11633ab550
>  0x7f110c000156:  c6 45 fc 01              movb     $1, -4(%rbp)
>  0x7f110c00015a:  8b 9d f8 da ff ff        movl     -0x2508(%rbp), %ebx
>  0x7f110c000160:  c1 e3 03                 shll     $3, %ebx
>  0x7f110c000163:  48 63 db                 movslq   %ebx, %rbx
>  0x7f110c000166:  49 bc b0 53 34 cb c2 55  movabsq  $0x55c2cb3453b0, %r12
>  0x7f110c00016e:  00 00
>  0x7f110c000170:  49 03 dc                 addq     %r12, %rbx
>  0x7f110c000173:  4c 8b 2b                 movq     (%rbx), %r13
>  0x7f110c000176:  49 ff c5                 incq     %r13
>  0x7f110c000179:  4c 89 2b                 movq     %r13, (%rbx)
>  0x7f110c00017c:  8b 9d f8 da ff ff        movl     -0x2508(%rbp), %ebx
>  0x7f110c000182:  c1 e3 03                 shll     $3, %ebx
>  0x7f110c000185:  48 63 db                 movslq   %ebx, %rbx
>  0x7f110c000188:  49 03 dc                 addq     %r12, %rbx
>  0x7f110c00018b:  48 8b 1b                 movq     (%rbx), %rbx
>  0x7f110c00018e:  48 83 fb 15              cmpq     $0x15, %rbx
>  0x7f110c000192:  0f 85 10 00 00 00        jne      0x7f110c0001a8
>  0x7f110c000198:  8b bd f8 da ff ff        movl     -0x2508(%rbp), %edi
>  0x7f110c00019e:  be f0 ff ff ff           movl     $0xfffffff0, %esi
>  0x7f110c0001a3:  e8 78 62 3a 57           callq    0x7f11633a6420
>  0x7f110c0001a8:  c7 45 54 00 f0 00 00     movl     $0xf000, 0x54(%rbp)
>  0x7f110c0001af:  c7 45 58 00 00 0f 00     movl     $0xf0000, 0x58(%rbp)
>  0x7f110c0001b6:  c7 45 20 5b e0 00 00     movl     $0xe05b, 0x20(%rbp)
>  0x7f110c0001bd:  48 8b fd                 movq     %rbp, %rdi
>  0x7f110c0001c0:  ff 15 12 00 00 00        callq    *0x12(%rip)
>  0x7f110c0001c6:  ff e0                    jmpq     *%rax
>  0x7f110c0001c8:  48 8d 05 74 fe ff ff     leaq     -0x18c(%rip), %rax
>  0x7f110c0001cf:  e9 44 fe ff ff           jmp      0x7f110c000018
>    -- tb slow paths + alignment
>  0x7f110c0001d4:  90                       nop      
>  0x7f110c0001d5:  90                       nop      
>  0x7f110c0001d6:  90                       nop      
>  0x7f110c0001d7:  90                       nop      
>    data: [size=8]
>  0x7f110c0001d8:  .quad  0x000055c2ba75abc0
>
>  before continuing to execute.
>
>  >
>  >
>  >>
>  >> On Mon, Sep 2, 2024, 16:08 Alex Bennée <[e-mail]> wrote:
>  >>
>  >>  Elisha Hollander <[e-mail]> writes:
>  >>
>  >>  > But for qemu_plugin_update_ns
>  >>  >
>  >>  > On Mon, Sep 2, 2024, 15:38 Elisha Hollander <[e-mail]> wrote:
>  >>  >
>  >>  >  Just checked with 9.0.2 it it still gives the error...
>  >>  >
>  >>  >  On Wed, Aug 28, 2024, 14:05 Alex Bennée <[e-mail]> wrote:
>  >>  >
>  >>  >  Elisha Hollander <[e-mail]> writes:
>  >>  >
>  >>  >  > Although it gives `undefined symbol: qemu_plugin_scoreboard_free`. But
>  >>  >  > probably I messed something up...
>  >>  >
>  >>  >  Are you using an older QEMU? We should trigger an API warning if they
>  >>  >  are mismatched but maybe thats not working.
>  >>  >
>  >>  >  >
>  >>  >  > On Tue, Aug 27, 2024, 14:59 Elisha Hollander <[e-mail]> wrote:
>  >>  >  >
>  >>  >  >  Oh nice, I didn't know that
>  >>  >  >
>  >>  >  >  On Tue, Aug 27, 2024, 12:39 Alex Bennée <[e-mail]> wrote:
>  >>  >  >
>  >>  >  >  Elisha Hollander <[e-mail]> writes:
>  >>  >  >
>  >>  >  >  > Signed-off-by: Elisha Hollander <[e-mail]>
>  >>  >  >
>  >>  >  >  What is the use-case for this patch?
>  >>  >  >
>  >>  >  >  If you are simply looking to slow the emulated system down please have a
>  >>  >  >  look at:
>  >>  >  >
>  >>  >  >    https://qemu.readthedocs.io/en/master/about/emulation.html#limit-instructions-per-second
>  >>  >  >
>  >>  >  >  which uses the plugin system to limit the run rate and sleep if its
>  >>  >  >  running too fast. The longer term goal is to deprecate the icount clock
>  >>  >  >  alignment feature from the core code and leave icount to just provide
>  >>  >  >  the deterministic execution needed for record/replay and reverse
>  >>  >  >  debugging.
>  >>  >  >
>  >>  >  >  > ---
>  >>  >  >  >  accel/tcg/cpu-exec.c      | 4 +---
>  >>  >  >  >  accel/tcg/icount-common.c | 4 ++--
>  >>  >  >  >  2 files changed, 3 insertions(+), 5 deletions(-)
>  >>  >  >  >
>  >>  >  >  > diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
>  >>  >  >  > index 8163295f34..4c2baf8ed4 100644
>  >>  >  >  > --- a/accel/tcg/cpu-exec.c
>  >>  >  >  > +++ b/accel/tcg/cpu-exec.c
>  >>  >  >  > @@ -95,11 +95,10 @@ static void align_clocks(SyncClocks *sc, CPUState *cpu)
>  >>  >  >  >  static void print_delay(const SyncClocks *sc)
>  >>  >  >  >  {
>  >>  >  >  >      static float threshold_delay;
>  >>  >  >  > -    static int64_t last_realtime_clock;
>  >>  >  >  >      static int nb_prints;
>  >>  >  >  >  
>  >>  >  >  >      if (icount_align_option &&
>  >>  >  >  > -        sc->realtime_clock - last_realtime_clock >= MAX_DELAY_PRINT_RATE &&
>  >>  >  >  > +        sc->diff_clk >= MAX_DELAY_PRINT_RATE &&
>  >>  >  >  >          nb_prints < MAX_NB_PRINTS) {
>  >>  >  >  >          if ((-sc->diff_clk / (float)1000000000LL > threshold_delay) ||
>  >>  >  >  >              (-sc->diff_clk / (float)1000000000LL <
>  >>  >  >  > @@ -109,7 +108,6 @@ static void print_delay(const SyncClocks *sc)
>  >>  >  >  >                          threshold_delay - 1,
>  >>  >  >  >                          threshold_delay);
>  >>  >  >  >              nb_prints++;
>  >>  >  >  > -            last_realtime_clock = sc->realtime_clock;
>  >>  >  >  >          }
>  >>  >  >  >      }
>  >>  >  >  >  }
>  >>  >  >  > diff --git a/accel/tcg/icount-common.c b/accel/tcg/icount-common.c
>  >>  >  >  > index 8d3d3a7e9d..f07f8baf4d 100644
>  >>  >  >  > --- a/accel/tcg/icount-common.c
>  >>  >  >  > +++ b/accel/tcg/icount-common.c
>  >>  >  >  > @@ -46,8 +46,8 @@
>  >>  >  >  >   * is TCG-specific, and does not need to be built for other accels.
>  >>  >  >  >   */
>  >>  >  >  >  static bool icount_sleep = true;
>  >>  >  >  > -/* Arbitrarily pick 1MIPS as the minimum allowable speed.  */
>  >>  >  >  > -#define MAX_ICOUNT_SHIFT 10
>  >>  >  >  > +/* Arbitrarily pick the minimum allowable speed.  */
>  >>  >  >  > +#define MAX_ICOUNT_SHIFT 30
>  >>  >  >  >  
>  >>  >  >  >  /* Do not count executed instructions */
>  >>  >  >  >  ICountMode use_icount = ICOUNT_DISABLED;
>  >>  >  >
>  >>  >  >  -- 
>  >>  >  >  Alex Bennée
>  >>  >  >  Virtualisation Tech Lead @ Linaro
>  >>  >
>  >>  >  -- 
>  >>  >  Alex Bennée
>  >>  >  Virtualisation Tech Lead @ Linaro
>  >>
>  >>  Can you give me your command line please?
>  >>
>  >>  -- 
>  >>  Alex Bennée
>  >>  Virtualisation Tech Lead @ Linaro
>
>  -- 
>  Alex Bennée
>  Virtualisation Tech Lead @ Linaro

Patch

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 8163295f34..4c2baf8ed4 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -95,11 +95,10 @@  static void align_clocks(SyncClocks *sc, CPUState *cpu)
 static void print_delay(const SyncClocks *sc)
 {
     static float threshold_delay;
-    static int64_t last_realtime_clock;
     static int nb_prints;
 
     if (icount_align_option &&
-        sc->realtime_clock - last_realtime_clock >= MAX_DELAY_PRINT_RATE &&
+        sc->diff_clk >= MAX_DELAY_PRINT_RATE &&
         nb_prints < MAX_NB_PRINTS) {
         if ((-sc->diff_clk / (float)1000000000LL > threshold_delay) ||
             (-sc->diff_clk / (float)1000000000LL <
@@ -109,7 +108,6 @@  static void print_delay(const SyncClocks *sc)
                         threshold_delay - 1,
                         threshold_delay);
             nb_prints++;
-            last_realtime_clock = sc->realtime_clock;
         }
     }
 }
diff --git a/accel/tcg/icount-common.c b/accel/tcg/icount-common.c
index 8d3d3a7e9d..f07f8baf4d 100644
--- a/accel/tcg/icount-common.c
+++ b/accel/tcg/icount-common.c
@@ -46,8 +46,8 @@ 
  * is TCG-specific, and does not need to be built for other accels.
  */
 static bool icount_sleep = true;
-/* Arbitrarily pick 1MIPS as the minimum allowable speed.  */
-#define MAX_ICOUNT_SHIFT 10
+/* Arbitrarily pick the minimum allowable speed.  */
+#define MAX_ICOUNT_SHIFT 30
 
 /* Do not count executed instructions */
 ICountMode use_icount = ICOUNT_DISABLED;