[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH 40/40] contrib/plugins: optimise the register value tracking
From: |
Alex Bennée |
Subject: |
[PATCH 40/40] contrib/plugins: optimise the register value tracking |
Date: |
Thu, 21 Dec 2023 10:38:18 +0000 |
This adds an additional flag which attempts to optimise the register
tracking by only instrumenting instructions which are likely to change
its value. This relies on the disassembler showing up the register
names in disassembly so is only enabled when asked for.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
docs/devel/tcg-plugins.rst | 10 +-
contrib/plugins/execlog.c | 189 ++++++++++++++++++++++++++++++-------
2 files changed, 165 insertions(+), 34 deletions(-)
diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst
index 3a0962723d7..fa7421279f5 100644
--- a/docs/devel/tcg-plugins.rst
+++ b/docs/devel/tcg-plugins.rst
@@ -503,7 +503,15 @@ registers with multiple ``reg`` options. You can also use
glob style matching if
$ qemu-system-arm $(QEMU_ARGS) \
-plugin ./contrib/plugins/libexeclog.so,reg=\*_el2,reg=sp -d plugin
-Be aware that each additional register to check will slow down execution quite
considerably.
+Be aware that each additional register to check will slow down
+execution quite considerably. You can optimise the number of register
+checks done by using the rdisas option. This will only instrument
+instructions that mention the registers in question in disassembly.
+This is not foolproof as some instructions implicitly change
+instructions. You can use the ifilter to catch these cases:
+
+ $ qemu-system-arm $(QEMU_ARGS) \
+ -plugin
./contrib/plugins/libexeclog.so,ifilter=msr,ifilter=blr,reg=x30,reg=\*_el1,rdisas=on
- contrib/plugins/cache.c
diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
index 74fbf7c0e60..f88e5acab6c 100644
--- a/contrib/plugins/execlog.c
+++ b/contrib/plugins/execlog.c
@@ -27,6 +27,7 @@ typedef struct CPU {
GString *last_exec;
/* Ptr array of Register */
GPtrArray *registers;
+ int index;
} CPU;
QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
@@ -38,6 +39,9 @@ static GRWLock expand_array_lock;
static GPtrArray *imatches;
static GArray *amatches;
static GPtrArray *rmatches;
+static bool disas_assist;
+static GMutex add_reg_name_lock;
+static GPtrArray *all_reg_names;
/**
* Add memory read or write information to current instruction log
@@ -72,9 +76,14 @@ static void vcpu_mem(unsigned int cpu_index,
qemu_plugin_meminfo_t info,
}
/**
- * Log instruction execution
+ * Log instruction execution, outputting the last one.
+ *
+ * vcpu_insn_exec() is a copy and paste of vcpu_insn_exec_with_regs()
+ * without the checking of register values when we've attempted to
+ * optimise with disas_assist.
*/
-static void vcpu_insn_exec(unsigned int cpu_index, void *udata)
+
+static CPU *get_cpu(int cpu_index)
{
CPU *cpu;
@@ -83,39 +92,87 @@ static void vcpu_insn_exec(unsigned int cpu_index, void
*udata)
cpu = &cpus[cpu_index];
g_rw_lock_reader_unlock(&expand_array_lock);
+ return cpu;
+}
+
+static void insn_check_regs(CPU *cpu) {
+ for (int n = 0; n < cpu->registers->len; n++) {
+ Register *reg = cpu->registers->pdata[n];
+ int sz;
+
+ g_byte_array_set_size(reg->new, 0);
+ sz = qemu_plugin_read_register(cpu->index, reg->handle, reg->new);
+ g_assert(sz == reg->last->len);
+
+ if (memcmp(reg->last->data, reg->new->data, sz)) {
+ GByteArray *temp = reg->last;
+ g_string_append_printf(cpu->last_exec, ", %s -> ", reg->name);
+ /* TODO: handle BE properly */
+ for (int i = sz; i >= 0; i--) {
+ g_string_append_printf(cpu->last_exec, "%02x",
+ reg->new->data[i]);
+ }
+ reg->last = reg->new;
+ reg->new = temp;
+ }
+ }
+}
+
+/* Log last instruction while checking registers */
+static void vcpu_insn_exec_with_regs(unsigned int cpu_index, void *udata)
+{
+ CPU *cpu = get_cpu(cpu_index);
+
/* Print previous instruction in cache */
if (cpu->last_exec->len) {
if (cpu->registers) {
- for (int n = 0; n < cpu->registers->len; n++) {
- Register *reg = cpu->registers->pdata[n];
- int sz;
-
- g_byte_array_set_size(reg->new, 0);
- sz = qemu_plugin_read_register(cpu_index, reg->handle,
reg->new);
- g_assert(sz == reg->last->len);
-
- if (memcmp(reg->last->data, reg->new->data, sz)) {
- GByteArray *temp = reg->last;
- g_string_append_printf(cpu->last_exec, ", %s -> ",
reg->name);
- /* TODO: handle BE properly */
- for (int i = sz; i >= 0; i--) {
- g_string_append_printf(cpu->last_exec, "%02x",
- reg->new->data[i]);
- }
- reg->last = reg->new;
- reg->new = temp;
- }
- }
+ insn_check_regs(cpu);
+ }
+
+ qemu_plugin_outs(cpu->last_exec->str);
+ qemu_plugin_outs("\n");
+ }
+
+ /* Store new instruction in cache */
+ /* vcpu_mem will add memory access information to last_exec */
+ g_string_printf(cpu->last_exec, "%u, ", cpu_index);
+ g_string_append(cpu->last_exec, (char *)udata);
+}
+
+/* Log last instruction while checking registers, ignore next */
+static void vcpu_insn_exec_only_regs(unsigned int cpu_index, void *udata)
+{
+ CPU *cpu = get_cpu(cpu_index);
+
+ /* Print previous instruction in cache */
+ if (cpu->last_exec->len) {
+ if (cpu->registers) {
+ insn_check_regs(cpu);
}
qemu_plugin_outs(cpu->last_exec->str);
qemu_plugin_outs("\n");
}
+ /* reset */
+ cpu->last_exec->len = 0;
+}
+
+/* Log last instruction without checking regs, setup next */
+static void vcpu_insn_exec(unsigned int cpu_index, void *udata)
+{
+ CPU *cpu = get_cpu(cpu_index);
+
+ /* Print previous instruction in cache */
+ if (cpu->last_exec->len) {
+ qemu_plugin_outs(cpu->last_exec->str);
+ qemu_plugin_outs("\n");
+ }
+
/* Store new instruction in cache */
/* vcpu_mem will add memory access information to last_exec */
- g_string_printf(cpus[cpu_index].last_exec, "%u, ", cpu_index);
- g_string_append(cpus[cpu_index].last_exec, (char *)udata);
+ g_string_printf(cpu->last_exec, "%u, ", cpu_index);
+ g_string_append(cpu->last_exec, (char *)udata);
}
/**
@@ -128,6 +185,8 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct
qemu_plugin_tb *tb)
{
struct qemu_plugin_insn *insn;
bool skip = (imatches || amatches);
+ bool check_regs_this = rmatches;
+ bool check_regs_next = false;
size_t n = qemu_plugin_tb_n_insns(tb);
for (size_t i = 0; i < n; i++) {
@@ -148,7 +207,8 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct
qemu_plugin_tb *tb)
/*
* If we are filtering we better check out if we have any
* hits. The skip "latches" so we can track memory accesses
- * after the instruction we care about.
+ * after the instruction we care about. Also enable register
+ * checking on the next instruction.
*/
if (skip && imatches) {
int j;
@@ -156,6 +216,7 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct
qemu_plugin_tb *tb)
char *m = g_ptr_array_index(imatches, j);
if (g_str_has_prefix(insn_disas, m)) {
skip = false;
+ check_regs_next = rmatches;
}
}
}
@@ -170,8 +231,38 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct
qemu_plugin_tb *tb)
}
}
+ /*
+ * Check the disassembly to see if a register we care about
+ * will be affected by this instruction. This relies on the
+ * dissembler doing something sensible for the registers we
+ * care about.
+ */
+ if (disas_assist && rmatches) {
+ check_regs_next = false;
+ gchar *args = g_strstr_len(insn_disas, -1, " ");
+ for (int n = 0; n < all_reg_names->len; n++) {
+ gchar *reg = g_ptr_array_index(all_reg_names, n);
+ if (g_strrstr(args, reg)) {
+ check_regs_next = true;
+ skip = false;
+ }
+ }
+ }
+
+ /*
+ * We now have 3 choices:
+ *
+ * Log this instruction normally
+ * Log this instruction checking for register changes
+ * Don't log this instruction but check for register changes from the
last one
+ */
+
if (skip) {
- g_free(insn_disas);
+ if (check_regs_this) {
+ qemu_plugin_register_vcpu_insn_exec_cb(insn,
+
vcpu_insn_exec_only_regs,
+ QEMU_PLUGIN_CB_R_REGS,
NULL);
+ }
} else {
uint32_t insn_opcode;
insn_opcode = *((uint32_t *)qemu_plugin_insn_data(insn));
@@ -184,15 +275,28 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct
qemu_plugin_tb *tb)
QEMU_PLUGIN_MEM_RW, NULL);
/* Register callback on instruction */
- qemu_plugin_register_vcpu_insn_exec_cb(
- insn, vcpu_insn_exec,
- rmatches ? QEMU_PLUGIN_CB_R_REGS : QEMU_PLUGIN_CB_NO_REGS,
- output);
+ if (check_regs_this) {
+ qemu_plugin_register_vcpu_insn_exec_cb(
+ insn, vcpu_insn_exec_with_regs,
+ QEMU_PLUGIN_CB_R_REGS,
+ output);
+ } else {
+ qemu_plugin_register_vcpu_insn_exec_cb(
+ insn, vcpu_insn_exec,
+ QEMU_PLUGIN_CB_NO_REGS,
+ output);
+ }
/* reset skip */
skip = (imatches || amatches);
}
+ /* set regs for next */
+ if (disas_assist && rmatches) {
+ check_regs_this = check_regs_next;
+ }
+
+ g_free(insn_disas);
}
}
@@ -200,10 +304,11 @@ static Register *init_vcpu_register(int vcpu_index,
qemu_plugin_reg_descriptor *desc)
{
Register *reg = g_new0(Register, 1);
+ g_autofree gchar *lower = g_utf8_strdown(desc->name, -1);
int r;
reg->handle = desc->handle;
- reg->name = g_strdup(desc->name);
+ reg->name = g_intern_string(lower);
reg->last = g_byte_array_new();
reg->new = g_byte_array_new();
@@ -213,7 +318,7 @@ static Register *init_vcpu_register(int vcpu_index,
return reg;
}
-static registers_init(int vcpu_index)
+static void registers_init(int vcpu_index)
{
GPtrArray *registers = g_ptr_array_new();
g_autoptr(GArray) reg_list = qemu_plugin_get_registers(vcpu_index);
@@ -228,9 +333,20 @@ static registers_init(int vcpu_index)
reg_list, qemu_plugin_reg_descriptor, r);
for (int p = 0; p < rmatches->len; p++) {
g_autoptr(GPatternSpec) pat =
g_pattern_spec_new(rmatches->pdata[p]);
- if (g_pattern_match_string(pat, rd->name)) {
+ g_autofree gchar *rd_lower = g_utf8_strdown(rd->name, -1);
+ if (g_pattern_match_string(pat, rd->name) ||
+ g_pattern_match_string(pat, rd_lower)) {
Register *reg = init_vcpu_register(vcpu_index, rd);
g_ptr_array_add(registers, reg);
+
+ /* we need a list of regnames at TB translation time */
+ if (disas_assist) {
+ g_mutex_lock(&add_reg_name_lock);
+ if (!g_ptr_array_find(all_reg_names, reg->name, NULL))
{
+ g_ptr_array_add(all_reg_names, reg->name);
+ }
+ g_mutex_unlock(&add_reg_name_lock);
+ }
}
}
}
@@ -254,6 +370,7 @@ static void vcpu_init(qemu_plugin_id_t id, unsigned int
vcpu_index)
if (vcpu_index >= num_cpus) {
cpus = g_realloc_n(cpus, vcpu_index + 1, sizeof(*cpus));
while (vcpu_index >= num_cpus) {
+ cpus[num_cpus].index = vcpu_index;
cpus[num_cpus].last_exec = g_string_new(NULL);
/* Any registers to track? */
@@ -336,6 +453,12 @@ QEMU_PLUGIN_EXPORT int
qemu_plugin_install(qemu_plugin_id_t id,
parse_vaddr_match(tokens[1]);
} else if (g_strcmp0(tokens[0], "reg") == 0) {
add_regpat(tokens[1]);
+ } else if (g_strcmp0(tokens[0], "rdisas") == 0) {
+ if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &disas_assist)) {
+ fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
+ return -1;
+ }
+ all_reg_names = g_ptr_array_new();
} else {
fprintf(stderr, "option parsing failed: %s\n", opt);
return -1;
--
2.39.2
- [PATCH 14/40] qtest: bump aspeed_smc-test timeout to 6 minutes, (continued)
- [PATCH 14/40] qtest: bump aspeed_smc-test timeout to 6 minutes, Alex Bennée, 2023/12/21
- [PATCH 13/40] qtest: bump qos-test timeout to 2 minutes, Alex Bennée, 2023/12/21
- [PATCH 12/40] qtest: bump boot-serial-test timeout to 3 minutes, Alex Bennée, 2023/12/21
- [PATCH 19/40] tests/fp: Bump fp-test-mulAdd test timeout to 3 minutes, Alex Bennée, 2023/12/21
- [PATCH 15/40] qtest: bump bios-table-test timeout to 9 minutes, Alex Bennée, 2023/12/21
- [PATCH 09/40] qtest: bump test-hmp timeout to 4 minutes, Alex Bennée, 2023/12/21
- [PATCH 39/40] contrib/plugins: extend execlog to track register changes, Alex Bennée, 2023/12/21
- [PATCH 36/40] gdbstub: expose api to find registers, Alex Bennée, 2023/12/21
- [PATCH 37/40] plugins: add an API to read registers, Alex Bennée, 2023/12/21
- [PATCH 38/40] contrib/plugins: fix imatch, Alex Bennée, 2023/12/21
- [PATCH 40/40] contrib/plugins: optimise the register value tracking,
Alex Bennée <=
- [PATCH 22/40] target/riscv: Remove misa_mxl validation, Alex Bennée, 2023/12/21
- [PATCH 23/40] target/riscv: Move misa_mxl_max to class, Alex Bennée, 2023/12/21
- [PATCH 25/40] target/arm: Use GDBFeature for dynamic XML, Alex Bennée, 2023/12/21
- [PATCH 27/40] target/riscv: Use GDBFeature for dynamic XML, Alex Bennée, 2023/12/21
- [PATCH 33/40] hw/core/cpu: Remove gdb_get_dynamic_xml member, Alex Bennée, 2023/12/21
- [PATCH 32/40] gdbstub: Infer number of core registers from XML, Alex Bennée, 2023/12/21
- [PATCH 35/40] plugins: Use different helpers when reading registers, Alex Bennée, 2023/12/21
- [PATCH 24/40] target/riscv: Validate misa_mxl_max only once, Alex Bennée, 2023/12/21
- [PATCH 31/40] gdbstub: Simplify XML lookup, Alex Bennée, 2023/12/21
- [PATCH 26/40] target/ppc: Use GDBFeature for dynamic XML, Alex Bennée, 2023/12/21