On 2026/10/09 1:40, Pierrick Bouvier wrote:
On 10/7/2026 11:11 PM, Akihiko Odaki wrote:On 2026/10/08 10:08, Pierrick Bouvier wrote:On 10/6/2026 11:01 PM, Akihiko Odaki wrote:A region of interest (ROI) often excludes the program startup and exit. For example, a benchmark may parse a large input and then run a computation kernel and the former may take longer than the latter; basic block vectors generated for the whole execution will not properly represent the latter.Add options to specify the beginning and the end of the ROI to bound basic block vector generation to the ROI. Signed-off-by: Akihiko Odaki <[email protected]> --- docs/about/emulation.rst | 8 +++++ contrib/plugins/bbv.c | 80 +++++++++++++++++++++++++++++++++++++ +++++++++-- 2 files changed, 86 insertions(+), 2 deletions(-) diff --git a/docs/about/emulation.rst b/docs/about/emulation.rst index c58149086cdd..0566171b517b 100644 --- a/docs/about/emulation.rst +++ b/docs/about/emulation.rst @@ -284,6 +284,14 @@ The bbv plugin allows you to generate basic block vectors for use with the * - Option - Description + * - begin=A + - The virtual address of the instruction that starts basic block vector + generation. Generation starts from the beginning of the execution if + not specified. + * - end=A + - The virtual address of the instruction that ends basic block vector + generation. Generation continues until the endo of the execution ifsmall typo here: s/endo/endThanks for catching the typo.+ not specified. * - interval=N - The interval to generate a basic block vector specified by the number of instructions (Default: N = 100000000) diff --git a/contrib/plugins/bbv.c b/contrib/plugins/bbv.c index e5caf5d9543b..a2a779f7639e 100644 --- a/contrib/plugins/bbv.c +++ b/contrib/plugins/bbv.c @@ -5,6 +5,8 @@ * SPDX-License-Identifier: GPL-2.0-or-later */ +#include <stdbool.h> +#include <stdint.h> #include <stdio.h> #include <glib.h> @@ -26,6 +28,10 @@ static GHashTable *bbs; static GRWLock bbs_lock; static char *filename; static struct qemu_plugin_scoreboard *vcpus; +static bool begin_present; +static uint64_t begin; +static bool end_present; +static uint64_t end; static int64_t interval = 100000000; static void plugin_exit(void *p) @@ -66,6 +72,7 @@ static void vcpu_init(unsigned int vcpu_index, void *userdata) Vcpu *vcpu = qemu_plugin_scoreboard_find(vcpus, vcpu_index); vcpu_filename = g_strdup_printf("%s.%u.bb", filename, vcpu_index); + vcpu->count = begin_present ? INT64_MIN : -interval;Not sure which semantic we are trying to achieve with -interval here. The original logic was to increment this count, until reaching interval, after what we would reset it by using vcpu_count -= interval, to keep the extra insn that went in after the threshold.The counter now starts at -interval and counts up to zero. Subtracting interval after emitting a vector still preserves the extra instructions. For example, with interval=100: initial count: -100 after 130 insns: 30 -> emit a vector subtract interval: -70 -> 70 insns until the next vectorvcpu->file = fopen(vcpu_filename, "w"); } @@ -102,6 +109,22 @@ static void vcpu_interval_exec(unsigned int vcpu_index, void *udata) fputc('\n', vcpu->file); } +static void vcpu_begin_insn_exec(unsigned int vcpu_index, void *userdata) +{ + GHashTableIter iter; + void *value; + + g_rw_lock_reader_lock(&bbs_lock); + g_hash_table_iter_init(&iter, bbs); + + while (g_hash_table_iter_next(&iter, NULL, &value)) { + qemu_plugin_u64_set(bb_count_u64(value), vcpu_index, 0); + } + + g_rw_lock_reader_unlock(&bbs_lock); + qemu_plugin_u64_set(count_u64(), vcpu_index, -interval); +} + static void vcpu_tb_trans(struct qemu_plugin_tb *tb, void *userdata) { uint64_t n_insns = qemu_plugin_tb_n_insns(tb); @@ -119,6 +142,19 @@ static void vcpu_tb_trans(struct qemu_plugin_tb *tb, void *userdata) } g_rw_lock_writer_unlock(&bbs_lock); + for (size_t idx = 0; idx < n_insns; idx++) { + struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, idx); + uint64_t insn_vaddr = qemu_plugin_insn_vaddr(insn); + + if (begin_present && insn_vaddr == begin) { + qemu_plugin_register_vcpu_insn_exec_cb( + insn, vcpu_begin_insn_exec, QEMU_PLUGIN_CB_NO_REGS, NULL); + } else if (end_present && insn_vaddr == end) { + qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu( + insn, QEMU_PLUGIN_INLINE_STORE_U64, count_u64(), INT64_MIN); + } + } + qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu( tb, QEMU_PLUGIN_INLINE_ADD_U64, count_u64(), n_insns); @@ -127,7 +163,32 @@ static void vcpu_tb_trans(struct qemu_plugin_tb *tb, void *userdata) qemu_plugin_register_vcpu_tb_exec_cond_cb( tb, vcpu_interval_exec, QEMU_PLUGIN_CB_NO_REGS, - QEMU_PLUGIN_COND_GE, count_u64(), interval, NULL); + QEMU_PLUGIN_COND_LT, count_u64(), INT64_MIN, NULL); +}is INT64_MIN used as an arbitrary large value when interpreted as unsigned immediate? If yes, that's confusing to the reader.The unsigned comparison count < (uint64_t)INT64_MIN is equivalent with the signed comparison (int64_t)count >= 0. That is why this particular value is used. Setting count to INT64_MIN suppresses output for the next 2^63 counted instructions, which is effectively disabled for this use. The begin callback clears the BB counters and sets count to -interval; end sets it back to INT64_MIN. The requested interval stays unchanged.Ok I see better. Even though it works, it's pretty confusing to be honest. Inline operatons works with unsigned values, so with conversion and wrap around, it's safe, but really not intuitive. I would prefer if you could refactor this to keep a zero initialized count value, and add a specific instrumentation path for begin, end logic (COND_LT, UINT64_MAX - N) to keep the same behavior of "infinite interval".
We cannot choose a separate instrumentation path based on whether execution is inside the ROI. Instrumentation is installed when a TB is translated, but the same TB may execute both inside and outside the ROI.
I will preserve this logic and add code comments for clarification. Regards, Akihiko Odaki
