| Message ID | 20230406022751.757980-3-fei2.wu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | accel/tcg/plugin: host insn size for plugin | expand |
Fei Wu <fei2.wu@intel.com> writes: > It's only valid when inline=false, otherwise it's default to 0. > > Signed-off-by: Fei Wu <fei2.wu@intel.com> > --- > contrib/plugins/hotblocks.c | 24 +++++++++++++++--------- > 1 file changed, 15 insertions(+), 9 deletions(-) > > diff --git a/contrib/plugins/hotblocks.c b/contrib/plugins/hotblocks.c > index 062200a7a4..c9716da7fe 100644 > --- a/contrib/plugins/hotblocks.c > +++ b/contrib/plugins/hotblocks.c > @@ -37,6 +37,8 @@ typedef struct { > uint64_t exec_count; > int trans_count; > unsigned long insns; > + void *p_host_insn_size; > + uint64_t host_insn_size; > } ExecCount; > > static gint cmp_exec_count(gconstpointer a, gconstpointer b) > @@ -59,13 +61,17 @@ static void plugin_exit(qemu_plugin_id_t id, void *p) > it = g_list_sort(counts, cmp_exec_count); > > if (it) { > - g_string_append_printf(report, "pc, tcount, icount, ecount\n"); > + g_string_append_printf(report, > + "host isize is only valid when inline=false\n" > + "pc, tcount, icount, ecount, host isize\n"); > > for (i = 0; i < limit && it->next; i++, it = it->next) { > ExecCount *rec = (ExecCount *) it->data; > - g_string_append_printf(report, "0x%016"PRIx64", %d, %ld, %"PRId64"\n", > + g_string_append_printf(report, "0x%016"PRIx64", %d, %ld, %"PRId64 > + ", %"PRIu64"\n", > rec->start_addr, rec->trans_count, > - rec->insns, rec->exec_count); > + rec->insns, rec->exec_count, > + rec->host_insn_size); > } > > g_list_free(it); > @@ -82,14 +88,13 @@ static void plugin_init(void) > > static void vcpu_tb_exec(unsigned int cpu_index, void *udata) > { > - ExecCount *cnt; > - uint64_t hash = (uint64_t) udata; > + ExecCount *cnt = (ExecCount *) udata; > > g_mutex_lock(&lock); > - cnt = (ExecCount *) g_hash_table_lookup(hotblocks, (gconstpointer) hash); > - /* should always succeed */ > - g_assert(cnt); > cnt->exec_count++; > + if (cnt->host_insn_size == 0) { > + cnt->host_insn_size = *((uint64_t *)cnt->p_host_insn_size); No - passing an internal TCG pointer across different phases of translation/execution is a definite no no. We explicitly state that handles are only valid for callbacks in the docs: Lifetime of the query handle ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Each callback provides an opaque anonymous information handle which can usually be further queried to find out information about a translation, instruction or operation. The handles themselves are only valid during the lifetime of the callback so it is important that any information that is needed is extracted during the callback and saved by the plugin. to avoid this sort of tangling of implementation details into the plugins. > + } > g_mutex_unlock(&lock); > } > > @@ -114,6 +119,7 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb) > cnt->start_addr = pc; > cnt->trans_count = 1; > cnt->insns = insns; > + cnt->p_host_insn_size = qemu_plugin_tb_host_insn_size(tb); > g_hash_table_insert(hotblocks, (gpointer) hash, (gpointer) cnt); > } > > @@ -125,7 +131,7 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb) > } else { > qemu_plugin_register_vcpu_tb_exec_cb(tb, vcpu_tb_exec, > QEMU_PLUGIN_CB_NO_REGS, > - (void *)hash); > + (void *)cnt); > } > }
diff --git a/contrib/plugins/hotblocks.c b/contrib/plugins/hotblocks.c index 062200a7a4..c9716da7fe 100644 --- a/contrib/plugins/hotblocks.c +++ b/contrib/plugins/hotblocks.c @@ -37,6 +37,8 @@ typedef struct { uint64_t exec_count; int trans_count; unsigned long insns; + void *p_host_insn_size; + uint64_t host_insn_size; } ExecCount; static gint cmp_exec_count(gconstpointer a, gconstpointer b) @@ -59,13 +61,17 @@ static void plugin_exit(qemu_plugin_id_t id, void *p) it = g_list_sort(counts, cmp_exec_count); if (it) { - g_string_append_printf(report, "pc, tcount, icount, ecount\n"); + g_string_append_printf(report, + "host isize is only valid when inline=false\n" + "pc, tcount, icount, ecount, host isize\n"); for (i = 0; i < limit && it->next; i++, it = it->next) { ExecCount *rec = (ExecCount *) it->data; - g_string_append_printf(report, "0x%016"PRIx64", %d, %ld, %"PRId64"\n", + g_string_append_printf(report, "0x%016"PRIx64", %d, %ld, %"PRId64 + ", %"PRIu64"\n", rec->start_addr, rec->trans_count, - rec->insns, rec->exec_count); + rec->insns, rec->exec_count, + rec->host_insn_size); } g_list_free(it); @@ -82,14 +88,13 @@ static void plugin_init(void) static void vcpu_tb_exec(unsigned int cpu_index, void *udata) { - ExecCount *cnt; - uint64_t hash = (uint64_t) udata; + ExecCount *cnt = (ExecCount *) udata; g_mutex_lock(&lock); - cnt = (ExecCount *) g_hash_table_lookup(hotblocks, (gconstpointer) hash); - /* should always succeed */ - g_assert(cnt); cnt->exec_count++; + if (cnt->host_insn_size == 0) { + cnt->host_insn_size = *((uint64_t *)cnt->p_host_insn_size); + } g_mutex_unlock(&lock); } @@ -114,6 +119,7 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb) cnt->start_addr = pc; cnt->trans_count = 1; cnt->insns = insns; + cnt->p_host_insn_size = qemu_plugin_tb_host_insn_size(tb); g_hash_table_insert(hotblocks, (gpointer) hash, (gpointer) cnt); } @@ -125,7 +131,7 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb) } else { qemu_plugin_register_vcpu_tb_exec_cb(tb, vcpu_tb_exec, QEMU_PLUGIN_CB_NO_REGS, - (void *)hash); + (void *)cnt); } }
It's only valid when inline=false, otherwise it's default to 0. Signed-off-by: Fei Wu <fei2.wu@intel.com> --- contrib/plugins/hotblocks.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-)