On 10/12/26 7:00 AM, Akihiko Odaki wrote:
On 2026/10/09 15:37, Pierrick Bouvier wrote:
On 10/8/26 8:33 PM, Akihiko Odaki wrote:
On 2026/10/09 11:30, Pierrick Bouvier wrote:
On 10/8/2026 5:57 PM, Akihiko Odaki wrote:
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 if
small typo here: s/endo/end
Thanks 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
vector
vcpu->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
beginEmulation
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.
I took some time to read again how bbv works. Indeed, my idea to
modify
interval was a non sense, and not what is expected.
The current instrumentation per tb is always done unconditionally,
and
the only need thing to do is to snapshot bbs hashtable to keep
track of
the correct data only when being inside the ROI. The rest of existing
logic stays the same.
- On enter, set vcpu->count to 0 (fresh start) and restore bb
table from
previous exit (empty table if there was no exit yet).
- On exit, duplicate the bb table to allow to restore it next time we
enter in ROI.
Key idea is that we keep on adding things in bb table, but save/
restore
when leaving/entering ROI.
I think for robustness you should also add detection of those
expected
transitions: Out of ROI -> Enter ROI -> Exit ROI -> Enter ROI -> ...
Nothing is currently done to detect if if we meet enter/exit address
multiple times, without having the correct sequence. I don't know
if we
want to report an error or handle reentrance correctly in this case.
Also, should count be reset on each entrance, or do we want to keep
track of it also, and save/restore that on exit/enter? I tend to
think
the idea is create a "virtual" interval than only is alive when being
inside the ROI. In this case, we should keep track of this value
also.
The current design that resets the bb table is intentional. A
"virtual"
interval spanning multiple ROIs would be misleading because it mixes
different sections of execution.
Ok.
I don't think we need to add a special handling to enforce a
particular
order of transitions. This patch performs the same action for the
begin
and end addresses and that makes the behavior predictable.
Perhaps we can extend the documentation to clarify the "begin"
marker to
tell that it resets the bb table, making the behavior clear regardless
of transition order.
I'm fine to keep a simple model, but I'll ask you to remove the logic
involving negative numbers, it's just too confusing. Please remove:
vcpu->count = begin_present ? INT64_MIN : -interval;
and
QEMU_PLUGIN_COND_LT, count_u64(), INT64_MIN, NULL);
Instead, let's add a new boolean "tracking_region" in struct Vcpu, that
is set to true when entering ROI (or true by default if no begin
parameter was given).
The flag may look simpler in isolation, but streamlining the
interactions with the counter requires more careful thought. Please see
below.
Then, modify instrumentation to replace:
```
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);
```
with:
```
else if (end_present && insn_vaddr == end) {
qemu_plugin_register_vcpu_insn_exec_cb(insn, vcpu_exit_region,
QEMU_PLUGIN_CB_NO_REGS, NULL);
}
static void vcpu_exit_region(unsigned int vcpu_index, void *userdata)
{
Vcpu *vcpu = qemu_plugin_scoreboard_find(vcpus, vcpu_index);
vcpu_interval_exec(vcpu_index, NULL);
vcpu->tracking_region = false;
}
```
vcpu_exit_region should only disable tracking. Calling
vcpu_interval_exec directly bypasses its count >= interval precondition:
it subtracts interval even when count is smaller, wrapping the unsigned
counter, and emits a partial vector or potentially an empty one.
I'm fine with it. I was unsure if you wanted to dump the partial data,
aggregate it when reentering the section, or simply discard it.
With a discard, it's even simpler, just need to set tracking_region to
false.
In addition, let's rename s/vcpu_begin_insn_exec/vcpu_enter_region, to
be consistent and explicit on what it's doing.
And finally, the last piece:
static void vcpu_interval_exec(unsigned int vcpu_index, void *udata)
{
Vcpu *vcpu = qemu_plugin_scoreboard_find(vcpus, vcpu_index);
if (!vcpu->tracking) {
return;
}
...
The early return must come after vcpu->count -= interval. Otherwise,
once count reaches interval outside the ROI, it remains above the
threshold and invokes the callback on every subsequent TB, rather than
once per interval.
Right, thanks for catching it.
The impact performance of calling vcpu_interval_exec + one if every N
instructions is invisible, and at least, it's a piece of code we can
read in 6 months, and understand in less than 2 min without any
additional doc.
I agree the performance impact is negligible, but at least you need to
consider about that to be sure. Adding another flag requires those
additional considerations.
More broadly, I don't think it's ideal to introduce application-specific
logic for this. Looking at the bigger picture, a counting timer with
variable expiration is a common pattern that should ideally be supported
generally.
An alternative option is to have signed integer variants of
qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu() and similar
functions to support for that. However, I don't think those are
necessary either. Taking at an even broader perspective, C allows
avoiding such specialization by letting unsigned variables hold signed
values in a well-defined manner; even the underlying implementation of
the inline op uses signed and unsigned types interchangeably.
If the argument is "C allows that with a defined behaviour", I don't
think it makes a point for me. There is a lot of well defined and
confusing code you can write.
Something like:
qemu_plugin_register_vcpu_tb_exec_cond_cb(
QEMU_PLUGIN_COND_LT, count_u64(), INT64_MIN, NULL);
with limit and counter being unsigned is not readable for me.
Although it might not be immediately obvious that an unsigned variable
is storing a signed value, adding a brief clarifying comment is still
simpler than introducing another variable overall.
From there I'll let you decide between introducing the flag, or
reimplement that with positive values (sticking to init count=0) only
if you really prefer this approach.
