On 10/7/19 11:28 AM, Alex Bennée wrote:
> From: "Vanderson M. do Rosario" <vanderson...@gmail.com>
>
> Adding tb_stats [start|pause|stop|filter] command to hmp.
> This allows controlling the collection of statistics.
> It is also possible to set the level of collection:
> all, jit, or exec.
>
> tb_stats filter allow to only collect statistics for the TB
> in the last_search list.
>
> The goal of this command is to allow the dynamic exploration
> of the TCG behavior and quality. Therefore, for now, a
> corresponding QMP command is not worthwhile.
>
> Acked-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
> Signed-off-by: Vanderson M. do Rosario <vanderson...@gmail.com>
> Message-Id: <20190829173437.5926-8-vanderson...@gmail.com>
> [AJB: fix authorship]
> Signed-off-by: Alex Bennée <alex.ben...@linaro.org>
>
> monitor: add tb_stats [start|pause|stop|filter] command to hmp.
>
> This allows controlling the collection of statistics.
> It is also possible to set the level of collection:
> all, jit, or exec.
>
> tb_stats filter allow to only collect statistics for the TB
> in the last_search list.
>
> The goal of this command is to allow the dynamic exploration
> of the TCG behavior and quality. Therefore, for now, a
> corresponding QMP command is not worthwhile.
>
> Acked-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
> Signed-off-by: Vanderson M. do Rosario <vanderson...@gmail.com>
> Message-Id: <20190829173437.5926-9-vanderson...@gmail.com>
> [AJB: fix authorship]
> Signed-off-by: Alex Bennée <alex.ben...@linaro.org>
> ---
Duplicated commit message contents.
> +void do_hmp_tbstats_safe(CPUState *cpu, run_on_cpu_data icmd)
Deserves a comment "Run via async_safe_run_on_cpu", as otherwise the "safe"
part of the function name is fairly obscure.
> + char *slevel = (char *) qdict_get_try_str(qdict, "level");
> + uint32_t level = TB_EXEC_STATS | TB_JIT_STATS | TB_JIT_TIME;
> + if (slevel) {
> + if (strcmp(slevel, "jit") == 0) {
> + level = TB_JIT_STATS;
> + } else if (strcmp(slevel, "exec") == 0) {
> + level = TB_EXEC_STATS;
> + } else if (strcmp(slevel, "time") == 0) {
> + level = TB_JIT_TIME;
> + }
> + }
No support for "+" or "all" as we do for the command-line?
> +static void reset_tbstats_flag(void *p, uint32_t hash, void *userp)
> +{
> + uint32_t flag = *((int *)userp);
Type cast doesn't match...
> + TBStatistics *tbs = p;
> + tbs->stats_enabled = flag;
> +}
> +
> +void set_tbstats_flags(uint32_t flag)
> +{
> + /* iterate over tbstats setting their flag as TB_NOTHING */
> + qht_iter(&tb_ctx.tb_stats, reset_tbstats_flag, &flag);
> +}
... here.
It does make me wonder if you'd do just as well with (void *)(uintptr_t)flag
and avoid the indirection. It is an opaque quantity shared only between these
two functions.
> static void hmp_info_profile(Monitor *mon, const QDict *qdict)
> {
> +#ifdef CONFIG_TCG
> dump_jit_exec_time_info(dev_time);
> dev_time = 0;
> +#else
> + error_report("TCG should be enabled!");
> +#endif
> }
>
> /* Capture support */
> diff --git a/vl.c b/vl.c
> index f91bc6fb4c..781fddaf18 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1796,11 +1796,17 @@ static bool main_loop_should_exit(void)
>
> static void main_loop(void)
> {
> +#ifdef CONFIG_TCG
> uint64_t ti;
> +#endif
> while (!main_loop_should_exit()) {
> +#ifdef CONFIG_TCG
> ti = profile_getclock();
> +#endif
> main_loop_wait(false);
> +#ifdef CONFIG_TCG
> dev_time += profile_getclock() - ti;
> +#endif
> }
> }
All of this was CONFIG_PROFILE just a few patches ago. If you need this, then
you broke bisection with !defined(CONFIG_TCG) a few patches ago.
r~
