Re: [PATCH] util/log: flush TB cache when log level changes

["Alex Bennée" <alex.bennee@linaro.org>]

Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:

> Sometimes we need to collect the translation logs starting
> from some point of the execution. Some TB listings may
> be missed in this case, when blocks were translated before.
> This patch clears TB cache to allow re-translation of such
> code blocks.
>
> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
> ---
>  accel/tcg/translate-all.c |    8 ++++++++
>  include/sysemu/tcg.h      |    1 +
>  stubs/meson.build         |    1 +
>  stubs/tcg.c               |   12 ++++++++++++
>  util/log.c                |    3 +++
>  5 files changed, 25 insertions(+)
>  create mode 100644 stubs/tcg.c
>
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index e9de6ff9dd..3acb227c57 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -1461,6 +1461,14 @@ void tb_flush(CPUState *cpu)
>      }
>  }
>  
> +void tb_flush_all(void)
> +{
> +    CPUState *cpu;
> +    CPU_FOREACH(cpu) {
> +        tb_flush(cpu);
> +    }
> +}
> +

This isn't needed - tb_flush flushes all translations although it does
need to be executed in a CPU context to do so.

>  /*
>   * Formerly ifdef DEBUG_TB_CHECK. These debug functions are user-mode-only,
>   * so in order to prevent bit rot we compile them unconditionally in user-mode,
> diff --git a/include/sysemu/tcg.h b/include/sysemu/tcg.h
> index 00349fb18a..7415f11022 100644
> --- a/include/sysemu/tcg.h
> +++ b/include/sysemu/tcg.h
> @@ -9,6 +9,7 @@
>  #define SYSEMU_TCG_H
>  
>  void tcg_exec_init(unsigned long tb_size, int splitwx);
> +void tb_flush_all(void);
>  
>  #ifdef CONFIG_TCG
>  extern bool tcg_allowed;
> diff --git a/stubs/meson.build b/stubs/meson.build
> index 80b1d81a31..95e70f8542 100644
> --- a/stubs/meson.build
> +++ b/stubs/meson.build
> @@ -38,6 +38,7 @@ stub_ss.add(files('set-fd-handler.c'))
>  stub_ss.add(files('sysbus.c'))
>  stub_ss.add(files('target-get-monitor-def.c'))
>  stub_ss.add(files('target-monitor-defs.c'))
> +stub_ss.add(files('tcg.c'))
>  stub_ss.add(files('tpm.c'))
>  stub_ss.add(files('trace-control.c'))
>  stub_ss.add(files('uuid.c'))
> diff --git a/stubs/tcg.c b/stubs/tcg.c
> new file mode 100644
> index 0000000000..775a748c77
> --- /dev/null
> +++ b/stubs/tcg.c
> @@ -0,0 +1,12 @@
> +/*
> + * TCG stubs
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "sysemu/tcg.h"
> +
> +void tb_flush_all(void)
> +{
> +}
> diff --git a/util/log.c b/util/log.c
> index 2ee1500bee..2ff342a91b 100644
> --- a/util/log.c
> +++ b/util/log.c
> @@ -26,6 +26,7 @@
>  #include "trace/control.h"
>  #include "qemu/thread.h"
>  #include "qemu/lockable.h"
> +#include "sysemu/tcg.h"
>  
>  static char *logfilename;
>  static QemuMutex qemu_logfile_mutex;
> @@ -84,6 +85,8 @@ void qemu_set_log(int log_flags)
>  #ifdef CONFIG_TRACE_LOG
>      qemu_loglevel |= LOG_TRACE;
>  #endif
> +    tb_flush_all();
> +

I would call tb_flush(current_cpu) or first_cpu here. But two things:

 - I'm not sure you have a CPU at all times qemu_set_log is called
 - It seems overly aggressive to throw away all translations every time
   the log level is changed. I would define a mask in log.h and have
   something like:

  if (log_flags & LOG_TRANSLATION) {
      tb_flush();
  }

>      /*
>       * In all cases we only log if qemu_loglevel is set.
>       * Also:


-- 
Alex Bennée