* Re: [PATCH RFC 1/1] TCG insn.c: Implement counting specific addresses
2025-04-30 10:59 ` [PATCH RFC 1/1] TCG insn.c: Implement counting specific addresses steffen_hirschmann
@ 2025-05-20 14:13 ` Alex Bennée
2025-05-28 10:35 ` Steffen Hirschmann
2025-05-26 14:35 ` [PATCH RFC v2 " steffen_hirschmann
1 sibling, 1 reply; 5+ messages in thread
From: Alex Bennée @ 2025-05-20 14:13 UTC (permalink / raw)
To: steffen_hirschmann
Cc: qemu-devel, Pierrick Bouvier, Mahmoud Mandour, Alexandre Iooss
steffen_hirschmann@web.de writes:
> From: Steffen Hirschmann <steffen_hirschmann@web.de>
>
> This commit implements counting of executed instruction within certain
> virtual address ranges via libinsn.so
This seems reasonable. Do you have any specific use cases where this
information is useful?
> Signed-off-by: Steffen Hirschmann <steffen_hirschmann@web.de>
> ---
> docs/about/emulation.rst | 2 +
> tests/tcg/plugins/insn.c | 89 ++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 91 insertions(+)
>
> diff --git a/docs/about/emulation.rst b/docs/about/emulation.rst
> index a72591ee4d..1fd122bc50 100644
> --- a/docs/about/emulation.rst
> +++ b/docs/about/emulation.rst
> @@ -336,6 +336,8 @@ Behaviour can be tweaked with the following arguments:
> - Give a summary of the instruction sizes for the execution
> * - match=<string>
> - Only instrument instructions matching the string prefix
> + * - vaddr=<start>+<count>
> + - Counts executed instructions in this virtual address range. <start> and <count> must be in base 16
>
> The ``match`` option will show some basic stats including how many
> instructions have executed since the last execution. For
> diff --git a/tests/tcg/plugins/insn.c b/tests/tcg/plugins/insn.c
> index 0c723cb9ed..c6d5b07d05 100644
> --- a/tests/tcg/plugins/insn.c
> +++ b/tests/tcg/plugins/insn.c
> @@ -48,6 +48,14 @@ static GHashTable *match_insn_records;
> static GMutex match_hash_lock;
>
>
> +typedef struct {
> + uint64_t start;
> + uint64_t end;
> + qemu_plugin_u64 hits; /* Number of insn executed in this range */
> +} VaddrRange;
> +
> +static GArray *vaddr_ranges;
> +
> static Instruction * get_insn_record(const char *disas, uint64_t vaddr, Match *m)
> {
> g_autofree char *str_hash = g_strdup_printf("%"PRIx64" %s", vaddr, disas);
> @@ -187,6 +195,15 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
> }
> g_free(insn_disas);
> }
> +
> + for (int j = 0; j < vaddr_ranges->len; j++) {
> + VaddrRange *var = &g_array_index(vaddr_ranges, VaddrRange, j);
> + uint64_t vaddr = qemu_plugin_insn_vaddr(insn);
> + if (var->start <= vaddr && vaddr < var->end) {
> + qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
> + insn, QEMU_PLUGIN_INLINE_ADD_U64, var->hits, 1);
> + }
> + }
> }
> }
>
> @@ -246,6 +263,19 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
>
> g_array_free(matches, TRUE);
> g_array_free(sizes, TRUE);
> +
> + for (i = 0; i < vaddr_ranges->len; ++i) {
> + VaddrRange *var = &g_array_index(vaddr_ranges, VaddrRange, i);
> + uint64_t hits = qemu_plugin_u64_sum(var->hits);
> +
> + g_string_printf(out, "Vaddr range [0x%08lx, 0x%08lx): %"PRId64" hits\n",
> + var->start, var->end, hits);
> + qemu_plugin_outs(out->str);
> + qemu_plugin_scoreboard_free(var->hits.score);
> + }
> +
> +
> + g_array_free(vaddr_ranges, TRUE);
> }
>
>
> @@ -258,6 +288,48 @@ static void parse_match(char *match)
> g_array_append_val(matches, new_match);
> }
>
> +static void parse_vaddr(const char *arg)
> +{
> + char *vaddr = g_strdup(arg);
We can make this:
g_autofree char *vaddr = g_strdup(arg);
to save the g_free() later.
> + char *saveptr, *token1, *token2;
> + uint64_t start, len, end;
> + token1 = strtok_r(vaddr, "+", &saveptr);
> + token2 = strtok_r(NULL, "+", &saveptr);
> + if (!token1 || !token2) {
> + fprintf(stderr, "vaddr usage: vaddr=start+count (both base 16!)\n");
> + exit(1);
> + }
rather than manually messing about with strtok_r how about:
uint64_t start = 0, len = 0;
g_auto(GStrv) tokens = g_strsplit(vaddr, "+", 2);
if (tokens[0] && tokens[2]) {
start = g_ascii_strtoull(tokens[0], NULL, 16);
len = g_ascii_strtoull(tokens[1], NULL, 16);
}
if ((start == 0 || start == GUINT64_MAX) ||
(len == 0 || len == GUINT64_MAX)) {
fprintf(stderr, "vaddr usage: vaddr=start+count (both base 16!)\n");
return false;
}
Alternatively we already export qemu_plugin_bool_parse() so maybe we
could consider re-factoring qemu_set_dfilter_ranges() to extract the
parser and make it available as a helper?
> +
> + start = g_ascii_strtoull(token1, NULL, 16);
> + len = g_ascii_strtoull(token2, NULL, 16);
> + end = start + len;
> +
> + g_free(vaddr);
see above
> +
> + if (start == 0 || end == 0) {
> + fprintf(stderr, "vaddr usage: vaddr=start+count (both base 16!)\n");
> + exit(1);
exit is rude for a plugin, we should fall back to QEMU by returning -1
> + }
> +
> + if (UINT64_MAX - start < len) {
> + fprintf(stderr, "integer overflow in vaddr end address calculation."
> + " Specified vaddr=start+count incorrect.\n");
> + exit(1);
> + }
> +
> + g_autoptr(GString) out = g_string_new(NULL);
> + g_string_printf(out, "Registering new Vaddr range"
> + " start=0x%08lx len=0x%08lx end=0x%08lx\n", start, len, end);
> + qemu_plugin_outs(out->str);
Keep variable declarations at the top of a block, that said do we need
to echo here?
> +
> + VaddrRange new_vaddrrange = {
> + .start = start,
> + .end = end,
> + .hits = qemu_plugin_scoreboard_u64(
> + qemu_plugin_scoreboard_new(sizeof(uint64_t))) };
> + g_array_append_val(vaddr_ranges, new_vaddrrange);
> +}
> +
> QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
> const qemu_info_t *info,
> int argc, char **argv)
> @@ -265,6 +337,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
> matches = g_array_new(false, true, sizeof(Match));
> /* null terminated so 0 is not a special case */
> sizes = g_array_new(true, true, sizeof(unsigned long));
> + vaddr_ranges = g_array_new(false, true, sizeof(VaddrRange));
>
> for (int i = 0; i < argc; i++) {
> char *opt = argv[i];
> @@ -281,6 +354,8 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
> }
> } else if (g_strcmp0(tokens[0], "match") == 0) {
> parse_match(tokens[1]);
> + } else if (g_strcmp0(tokens[0], "vaddr") == 0) {
> + parse_vaddr(tokens[1]);
> } else if (g_strcmp0(tokens[0], "trace") == 0) {
> if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_trace)) {
> fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
> @@ -292,6 +367,20 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
> }
> }
>
> + /* Check for invalid parameter combinations */
> + if (vaddr_ranges->len > 0) {
> + if (matches->len > 0) {
> + fprintf(stderr, "match=... and vaddr=... are incompatible."
> + " Use only one of them.\n");
> + return -1;
> + }
> +
> + if (!do_inline) {
> + fprintf(stderr, "vaddr=... is only supported in conjunction with inline.\n");
> + return -1;
> + }
> + }
> +
> insn_count = qemu_plugin_scoreboard_u64(
> qemu_plugin_scoreboard_new(sizeof(uint64_t)));
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH RFC v2 1/1] TCG insn.c: Implement counting specific addresses
2025-04-30 10:59 ` [PATCH RFC 1/1] TCG insn.c: Implement counting specific addresses steffen_hirschmann
2025-05-20 14:13 ` Alex Bennée
@ 2025-05-26 14:35 ` steffen_hirschmann
1 sibling, 0 replies; 5+ messages in thread
From: steffen_hirschmann @ 2025-05-26 14:35 UTC (permalink / raw)
To: Alex Bennée, Alexandre Iooss, Mahmoud Mandour,
Pierrick Bouvier, open list:All patches CC here
Cc: Steffen Hirschmann
From: Steffen Hirschmann <steffen_hirschmann@web.de>
This commit implements counting of executed instruction within certain
virtual address ranges via libinsn.so
Signed-off-by: Steffen Hirschmann <steffen_hirschmann@web.de>
---
docs/about/emulation.rst | 2 +
tests/tcg/plugins/insn.c | 91 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 93 insertions(+)
diff --git a/docs/about/emulation.rst b/docs/about/emulation.rst
index a72591ee4d..1fd122bc50 100644
--- a/docs/about/emulation.rst
+++ b/docs/about/emulation.rst
@@ -336,6 +336,8 @@ Behaviour can be tweaked with the following arguments:
- Give a summary of the instruction sizes for the execution
* - match=<string>
- Only instrument instructions matching the string prefix
+ * - vaddr=<start>+<count>
+ - Counts executed instructions in this virtual address range. <start> and <count> must be in base 16
The ``match`` option will show some basic stats including how many
instructions have executed since the last execution. For
diff --git a/tests/tcg/plugins/insn.c b/tests/tcg/plugins/insn.c
index 0c723cb9ed..41c2a2ae32 100644
--- a/tests/tcg/plugins/insn.c
+++ b/tests/tcg/plugins/insn.c
@@ -48,6 +48,14 @@ static GHashTable *match_insn_records;
static GMutex match_hash_lock;
+typedef struct {
+ uint64_t start;
+ uint64_t end;
+ qemu_plugin_u64 hits; /* Number of insn executed in this range */
+} VaddrRange;
+
+static GArray *vaddr_ranges;
+
static Instruction * get_insn_record(const char *disas, uint64_t vaddr, Match *m)
{
g_autofree char *str_hash = g_strdup_printf("%"PRIx64" %s", vaddr, disas);
@@ -187,6 +195,15 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
}
g_free(insn_disas);
}
+
+ for (int j = 0; j < vaddr_ranges->len; j++) {
+ VaddrRange *var = &g_array_index(vaddr_ranges, VaddrRange, j);
+ uint64_t vaddr = qemu_plugin_insn_vaddr(insn);
+ if (var->start <= vaddr && vaddr < var->end) {
+ qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
+ insn, QEMU_PLUGIN_INLINE_ADD_U64, var->hits, 1);
+ }
+ }
}
}
@@ -246,6 +263,19 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
g_array_free(matches, TRUE);
g_array_free(sizes, TRUE);
+
+ for (i = 0; i < vaddr_ranges->len; ++i) {
+ VaddrRange *var = &g_array_index(vaddr_ranges, VaddrRange, i);
+ uint64_t hits = qemu_plugin_u64_sum(var->hits);
+
+ g_string_printf(out, "Vaddr range [0x%08lx, 0x%08lx): %"PRId64" hits\n",
+ var->start, var->end, hits);
+ qemu_plugin_outs(out->str);
+ qemu_plugin_scoreboard_free(var->hits.score);
+ }
+
+
+ g_array_free(vaddr_ranges, TRUE);
}
@@ -258,6 +288,48 @@ static void parse_match(char *match)
g_array_append_val(matches, new_match);
}
+/* Returns true on success. If parsing is unsuccessful or vaddr bounds are incorrect, returns false. */
+static bool parse_vaddr(const char *arg)
+{
+ g_autofree char *vaddr = g_strdup(arg);
+ char *endptr;
+ uint64_t start = 0, len = 0;
+ g_auto(GStrv) tokens = g_strsplit(vaddr, "+", 2);
+
+ if (tokens && tokens[0] && tokens[1]) {
+ start = g_ascii_strtoull(tokens[0], &endptr, 16);
+ if (start == G_MAXUINT64 /* uint64_t overflow */
+ || endptr == tokens[0] /* conversion failed */
+ || endptr != tokens[0] + strlen(tokens[0]) /* garbage data at the end */) {
+ fprintf(stderr, "vaddr usage: vaddr=start+count (both base 16!). 'start' address invalid or overflow\n");
+ return false;
+ }
+ len = g_ascii_strtoull(tokens[1], &endptr, 16);
+ if (len == G_MAXUINT64 || endptr == tokens[1] || endptr != tokens[1] + strlen(tokens[1])) {
+ fprintf(stderr, "vaddr usage: vaddr=start+count (both base 16!). 'count' invalid or overflow\n");
+ return false;
+ }
+ } else {
+ fprintf(stderr, "vaddr usage: vaddr=start+count (both base 16!)\n");
+ return false;
+ }
+
+ if (UINT64_MAX - start < len) {
+ fprintf(stderr, "integer overflow in vaddr end address calculation."
+ " Specified vaddr=start+count incorrect.\n");
+ return false;
+ }
+
+ VaddrRange new_vaddrrange = {
+ .start = start,
+ .end = start + len,
+ .hits = qemu_plugin_scoreboard_u64(
+ qemu_plugin_scoreboard_new(sizeof(uint64_t))) };
+ g_array_append_val(vaddr_ranges, new_vaddrrange);
+
+ return true;
+}
+
QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
const qemu_info_t *info,
int argc, char **argv)
@@ -265,6 +337,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
matches = g_array_new(false, true, sizeof(Match));
/* null terminated so 0 is not a special case */
sizes = g_array_new(true, true, sizeof(unsigned long));
+ vaddr_ranges = g_array_new(false, true, sizeof(VaddrRange));
for (int i = 0; i < argc; i++) {
char *opt = argv[i];
@@ -281,6 +354,10 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
}
} else if (g_strcmp0(tokens[0], "match") == 0) {
parse_match(tokens[1]);
+ } else if (g_strcmp0(tokens[0], "vaddr") == 0) {
+ if (!parse_vaddr(tokens[1])) {
+ return -1;
+ }
} else if (g_strcmp0(tokens[0], "trace") == 0) {
if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_trace)) {
fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
@@ -292,6 +369,20 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
}
}
+ /* Check for invalid parameter combinations */
+ if (vaddr_ranges->len > 0) {
+ if (matches->len > 0) {
+ fprintf(stderr, "match=... and vaddr=... are incompatible."
+ " Use only one of them.\n");
+ return -1;
+ }
+
+ if (!do_inline) {
+ fprintf(stderr, "vaddr=... is only supported in conjunction with inline.\n");
+ return -1;
+ }
+ }
+
insn_count = qemu_plugin_scoreboard_u64(
qemu_plugin_scoreboard_new(sizeof(uint64_t)));
--
2.47.2
^ permalink raw reply related [flat|nested] 5+ messages in thread