* [PATCH 0/5] TCG plugins new inline operations
@ 2024-02-29 5:53 Pierrick Bouvier
2024-02-29 5:53 ` [PATCH 1/5] plugins: prepare introduction of new inline ops Pierrick Bouvier
` (5 more replies)
0 siblings, 6 replies; 14+ messages in thread
From: Pierrick Bouvier @ 2024-02-29 5:53 UTC (permalink / raw)
To: qemu-devel
Cc: Mahmoud Mandour, Paolo Bonzini, Alexandre Iooss, Alex Bennée,
Pierrick Bouvier, Richard Henderson
This series implement two new operations for plugins:
- Store inline allows to write a specific value to a scoreboard.
- Conditional callback executes a callback only when a given condition is true.
The condition is evaluated inline.
It's possible to mix various inline operations (add, store) with conditional
callbacks, allowing efficient "trap" based counters.
It builds on top of new scoreboard API, introduced in the previous series.
Based-on: 20240229052506.933222-1-pierrick.bouvier@linaro.org
Pierrick Bouvier (5):
plugins: prepare introduction of new inline ops
plugins: add new inline op STORE_U64
tests/plugin/inline: add test for STORE_U64 inline op
plugins: conditional callbacks
tests/plugin/inline: add test for condition callback
include/qemu/plugin.h | 10 +-
include/qemu/qemu-plugin.h | 80 +++++++-
plugins/plugin.h | 9 +
accel/tcg/plugin-gen.c | 359 +++++++++++++++++++++++++++++++----
plugins/api.c | 76 +++++++-
plugins/core.c | 28 ++-
tests/plugin/inline.c | 128 ++++++++++++-
plugins/qemu-plugins.symbols | 2 +
8 files changed, 633 insertions(+), 59 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/5] plugins: prepare introduction of new inline ops
2024-02-29 5:53 [PATCH 0/5] TCG plugins new inline operations Pierrick Bouvier
@ 2024-02-29 5:53 ` Pierrick Bouvier
2024-02-29 5:53 ` [PATCH 2/5] plugins: add new inline op STORE_U64 Pierrick Bouvier
` (4 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Pierrick Bouvier @ 2024-02-29 5:53 UTC (permalink / raw)
To: qemu-devel
Cc: Mahmoud Mandour, Paolo Bonzini, Alexandre Iooss, Alex Bennée,
Pierrick Bouvier, Richard Henderson
Until now, only add_u64 was available, and all functions assumed this or
were named uniquely.
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
include/qemu/plugin.h | 2 +-
plugins/plugin.h | 1 +
accel/tcg/plugin-gen.c | 77 +++++++++++++++++++++---------------------
plugins/api.c | 23 ++++++++++---
plugins/core.c | 5 +--
5 files changed, 61 insertions(+), 47 deletions(-)
diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index 12a96cea2a4..33a7cbe910c 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -74,7 +74,7 @@ enum plugin_dyn_cb_type {
enum plugin_dyn_cb_subtype {
PLUGIN_CB_REGULAR,
PLUGIN_CB_REGULAR_R,
- PLUGIN_CB_INLINE,
+ PLUGIN_CB_INLINE_ADD_U64,
PLUGIN_N_CB_SUBTYPES,
};
diff --git a/plugins/plugin.h b/plugins/plugin.h
index 7c34f23cfcb..696b1fa38b0 100644
--- a/plugins/plugin.h
+++ b/plugins/plugin.h
@@ -70,6 +70,7 @@ struct qemu_plugin_ctx *plugin_id_to_ctx_locked(qemu_plugin_id_t id);
void plugin_register_inline_op_on_entry(GArray **arr,
enum qemu_plugin_mem_rw rw,
+ enum plugin_dyn_cb_subtype type,
enum qemu_plugin_op op,
qemu_plugin_u64 entry,
uint64_t imm);
diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index 8028786c7bb..494467e0833 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -81,7 +81,7 @@ enum plugin_gen_from {
enum plugin_gen_cb {
PLUGIN_GEN_CB_UDATA,
PLUGIN_GEN_CB_UDATA_R,
- PLUGIN_GEN_CB_INLINE,
+ PLUGIN_GEN_CB_INLINE_ADD_U64,
PLUGIN_GEN_CB_MEM,
PLUGIN_GEN_ENABLE_MEM_HELPER,
PLUGIN_GEN_DISABLE_MEM_HELPER,
@@ -127,11 +127,7 @@ static void gen_empty_udata_cb_no_rwg(void)
gen_empty_udata_cb(gen_helper_plugin_vcpu_udata_cb_no_rwg);
}
-/*
- * For now we only support addi_i64.
- * When we support more ops, we can generate one empty inline cb for each.
- */
-static void gen_empty_inline_cb(void)
+static void gen_empty_inline_cb_add_u64(void)
{
TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
TCGv_ptr cpu_index_as_ptr = tcg_temp_ebb_new_ptr();
@@ -219,9 +215,11 @@ static void plugin_gen_empty_callback(enum plugin_gen_from from)
gen_empty_mem_helper);
/* fall through */
case PLUGIN_GEN_FROM_TB:
+ /* emit inline op before any callback */
+ gen_wrapped(from, PLUGIN_GEN_CB_INLINE_ADD_U64,
+ gen_empty_inline_cb_add_u64);
gen_wrapped(from, PLUGIN_GEN_CB_UDATA, gen_empty_udata_cb_no_rwg);
gen_wrapped(from, PLUGIN_GEN_CB_UDATA_R, gen_empty_udata_cb_no_wg);
- gen_wrapped(from, PLUGIN_GEN_CB_INLINE, gen_empty_inline_cb);
break;
default:
g_assert_not_reached();
@@ -232,13 +230,14 @@ void plugin_gen_empty_mem_callback(TCGv_i64 addr, uint32_t info)
{
enum qemu_plugin_mem_rw rw = get_plugin_meminfo_rw(info);
+ /* emit inline op before any callback */
+ gen_plugin_cb_start(PLUGIN_GEN_FROM_MEM, PLUGIN_GEN_CB_INLINE_ADD_U64, rw);
+ gen_empty_inline_cb_add_u64();
+ tcg_gen_plugin_cb_end();
+
gen_plugin_cb_start(PLUGIN_GEN_FROM_MEM, PLUGIN_GEN_CB_MEM, rw);
gen_empty_mem_cb(addr, info);
tcg_gen_plugin_cb_end();
-
- gen_plugin_cb_start(PLUGIN_GEN_FROM_MEM, PLUGIN_GEN_CB_INLINE, rw);
- gen_empty_inline_cb();
- tcg_gen_plugin_cb_end();
}
static TCGOp *find_op(TCGOp *op, TCGOpcode opc)
@@ -436,9 +435,9 @@ static TCGOp *append_udata_cb(const struct qemu_plugin_dyn_cb *cb,
return op;
}
-static TCGOp *append_inline_cb(const struct qemu_plugin_dyn_cb *cb,
- TCGOp *begin_op, TCGOp *op,
- int *unused)
+static TCGOp *append_inline_cb_add_u64(const struct qemu_plugin_dyn_cb *cb,
+ TCGOp *begin_op, TCGOp *op,
+ int *unused)
{
char *ptr = cb->inline_insn.entry.score->data->data;
size_t elem_size = g_array_get_element_size(
@@ -538,9 +537,9 @@ inject_udata_cb(const GArray *cbs, TCGOp *begin_op)
}
static void
-inject_inline_cb(const GArray *cbs, TCGOp *begin_op, op_ok_fn ok)
+inject_inline_cb_add_u64(const GArray *cbs, TCGOp *begin_op, op_ok_fn ok)
{
- inject_cb_type(cbs, begin_op, append_inline_cb, ok);
+ inject_cb_type(cbs, begin_op, append_inline_cb_add_u64, ok);
}
static void
@@ -588,8 +587,9 @@ static void inject_mem_enable_helper(struct qemu_plugin_tb *ptb,
GArray *arr;
size_t n_cbs, i;
- cbs[0] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_REGULAR];
- cbs[1] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_INLINE];
+ /* emit inline op before any callback */
+ cbs[0] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_INLINE_ADD_U64];
+ cbs[1] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_REGULAR];
n_cbs = 0;
for (i = 0; i < ARRAY_SIZE(cbs); i++) {
@@ -655,10 +655,11 @@ static void plugin_gen_tb_udata_r(const struct qemu_plugin_tb *ptb,
inject_udata_cb(ptb->cbs[PLUGIN_CB_REGULAR_R], begin_op);
}
-static void plugin_gen_tb_inline(const struct qemu_plugin_tb *ptb,
- TCGOp *begin_op)
+static void plugin_gen_tb_inline_add_u64(const struct qemu_plugin_tb *ptb,
+ TCGOp *begin_op)
{
- inject_inline_cb(ptb->cbs[PLUGIN_CB_INLINE], begin_op, op_ok);
+ inject_inline_cb_add_u64(ptb->cbs[PLUGIN_CB_INLINE_ADD_U64],
+ begin_op, op_ok);
}
static void plugin_gen_insn_udata(const struct qemu_plugin_tb *ptb,
@@ -677,12 +678,12 @@ static void plugin_gen_insn_udata_r(const struct qemu_plugin_tb *ptb,
inject_udata_cb(insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_REGULAR_R], begin_op);
}
-static void plugin_gen_insn_inline(const struct qemu_plugin_tb *ptb,
- TCGOp *begin_op, int insn_idx)
+static void plugin_gen_insn_inline_add_u64(const struct qemu_plugin_tb *ptb,
+ TCGOp *begin_op, int insn_idx)
{
struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
- inject_inline_cb(insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_INLINE],
- begin_op, op_ok);
+ inject_inline_cb_add_u64(
+ insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_INLINE_ADD_U64], begin_op, op_ok);
}
static void plugin_gen_mem_regular(const struct qemu_plugin_tb *ptb,
@@ -692,14 +693,12 @@ static void plugin_gen_mem_regular(const struct qemu_plugin_tb *ptb,
inject_mem_cb(insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_REGULAR], begin_op);
}
-static void plugin_gen_mem_inline(const struct qemu_plugin_tb *ptb,
- TCGOp *begin_op, int insn_idx)
+static void plugin_gen_mem_inline_add_u64(const struct qemu_plugin_tb *ptb,
+ TCGOp *begin_op, int insn_idx)
{
- const GArray *cbs;
struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
-
- cbs = insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_INLINE];
- inject_inline_cb(cbs, begin_op, op_rw);
+ inject_inline_cb_add_u64(insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_INLINE_ADD_U64],
+ begin_op, op_rw);
}
static void plugin_gen_enable_mem_helper(struct qemu_plugin_tb *ptb,
@@ -748,8 +747,8 @@ static void pr_ops(void)
case PLUGIN_GEN_CB_UDATA:
type = "udata";
break;
- case PLUGIN_GEN_CB_INLINE:
- type = "inline";
+ case PLUGIN_GEN_CB_INLINE_ADD_U64:
+ type = "inline add u64";
break;
case PLUGIN_GEN_CB_MEM:
type = "mem";
@@ -799,8 +798,8 @@ static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
case PLUGIN_GEN_CB_UDATA_R:
plugin_gen_tb_udata_r(plugin_tb, op);
break;
- case PLUGIN_GEN_CB_INLINE:
- plugin_gen_tb_inline(plugin_tb, op);
+ case PLUGIN_GEN_CB_INLINE_ADD_U64:
+ plugin_gen_tb_inline_add_u64(plugin_tb, op);
break;
default:
g_assert_not_reached();
@@ -818,8 +817,8 @@ static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
case PLUGIN_GEN_CB_UDATA_R:
plugin_gen_insn_udata_r(plugin_tb, op, insn_idx);
break;
- case PLUGIN_GEN_CB_INLINE:
- plugin_gen_insn_inline(plugin_tb, op, insn_idx);
+ case PLUGIN_GEN_CB_INLINE_ADD_U64:
+ plugin_gen_insn_inline_add_u64(plugin_tb, op, insn_idx);
break;
case PLUGIN_GEN_ENABLE_MEM_HELPER:
plugin_gen_enable_mem_helper(plugin_tb, op, insn_idx);
@@ -837,8 +836,8 @@ static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
case PLUGIN_GEN_CB_MEM:
plugin_gen_mem_regular(plugin_tb, op, insn_idx);
break;
- case PLUGIN_GEN_CB_INLINE:
- plugin_gen_mem_inline(plugin_tb, op, insn_idx);
+ case PLUGIN_GEN_CB_INLINE_ADD_U64:
+ plugin_gen_mem_inline_add_u64(plugin_tb, op, insn_idx);
break;
default:
g_assert_not_reached();
diff --git a/plugins/api.c b/plugins/api.c
index 58e851effdb..e3bcacfa99a 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -55,6 +55,16 @@
#endif
#endif
+static enum plugin_dyn_cb_subtype op_to_cb_subtype(enum qemu_plugin_op op)
+{
+ switch (op) {
+ case QEMU_PLUGIN_INLINE_ADD_U64:
+ return PLUGIN_CB_INLINE_ADD_U64;
+ default:
+ g_assert_not_reached();
+ }
+}
+
/* Uninstall and Reset handlers */
void qemu_plugin_uninstall(qemu_plugin_id_t id, qemu_plugin_simple_cb_t cb)
@@ -108,8 +118,9 @@ void qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
uint64_t imm)
{
if (!tb->mem_only) {
- plugin_register_inline_op_on_entry(
- &tb->cbs[PLUGIN_CB_INLINE], 0, op, entry, imm);
+ enum plugin_dyn_cb_subtype type = op_to_cb_subtype(op);
+ plugin_register_inline_op_on_entry(&tb->cbs[type],
+ 0, type, op, entry, imm);
}
}
@@ -135,8 +146,9 @@ void qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
uint64_t imm)
{
if (!insn->mem_only) {
- plugin_register_inline_op_on_entry(
- &insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_INLINE], 0, op, entry, imm);
+ enum plugin_dyn_cb_subtype type = op_to_cb_subtype(op);
+ plugin_register_inline_op_on_entry(&insn->cbs[PLUGIN_CB_INSN][type],
+ 0, type, op, entry, imm);
}
}
@@ -162,8 +174,9 @@ void qemu_plugin_register_vcpu_mem_inline_per_vcpu(
qemu_plugin_u64 entry,
uint64_t imm)
{
+ enum plugin_dyn_cb_subtype type = op_to_cb_subtype(op);
plugin_register_inline_op_on_entry(
- &insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_INLINE], rw, op, entry, imm);
+ &insn->cbs[PLUGIN_CB_MEM][type], rw, type, op, entry, imm);
}
void qemu_plugin_register_vcpu_tb_trans_cb(qemu_plugin_id_t id,
diff --git a/plugins/core.c b/plugins/core.c
index 11ca20e6267..a641a366ef9 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -318,6 +318,7 @@ static struct qemu_plugin_dyn_cb *plugin_get_dyn_cb(GArray **arr)
void plugin_register_inline_op_on_entry(GArray **arr,
enum qemu_plugin_mem_rw rw,
+ enum plugin_dyn_cb_subtype type,
enum qemu_plugin_op op,
qemu_plugin_u64 entry,
uint64_t imm)
@@ -326,7 +327,7 @@ void plugin_register_inline_op_on_entry(GArray **arr,
dyn_cb = plugin_get_dyn_cb(arr);
dyn_cb->userp = NULL;
- dyn_cb->type = PLUGIN_CB_INLINE;
+ dyn_cb->type = type;
dyn_cb->rw = rw;
dyn_cb->inline_insn.entry = entry;
dyn_cb->inline_insn.op = op;
@@ -514,7 +515,7 @@ void qemu_plugin_vcpu_mem_cb(CPUState *cpu, uint64_t vaddr,
cb->f.vcpu_mem(cpu->cpu_index, make_plugin_meminfo(oi, rw),
vaddr, cb->userp);
break;
- case PLUGIN_CB_INLINE:
+ case PLUGIN_CB_INLINE_ADD_U64:
exec_inline_op(cb, cpu->cpu_index);
break;
default:
--
2.43.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/5] plugins: add new inline op STORE_U64
2024-02-29 5:53 [PATCH 0/5] TCG plugins new inline operations Pierrick Bouvier
2024-02-29 5:53 ` [PATCH 1/5] plugins: prepare introduction of new inline ops Pierrick Bouvier
@ 2024-02-29 5:53 ` Pierrick Bouvier
2024-02-29 5:53 ` [PATCH 3/5] tests/plugin/inline: add test for STORE_U64 inline op Pierrick Bouvier
` (3 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Pierrick Bouvier @ 2024-02-29 5:53 UTC (permalink / raw)
To: qemu-devel
Cc: Mahmoud Mandour, Paolo Bonzini, Alexandre Iooss, Alex Bennée,
Pierrick Bouvier, Richard Henderson
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
include/qemu/plugin.h | 1 +
include/qemu/qemu-plugin.h | 4 +-
accel/tcg/plugin-gen.c | 114 ++++++++++++++++++++++++++++++++++++-
plugins/api.c | 2 +
plugins/core.c | 4 ++
5 files changed, 120 insertions(+), 5 deletions(-)
diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index 33a7cbe910c..d92d64744e6 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -75,6 +75,7 @@ enum plugin_dyn_cb_subtype {
PLUGIN_CB_REGULAR,
PLUGIN_CB_REGULAR_R,
PLUGIN_CB_INLINE_ADD_U64,
+ PLUGIN_CB_INLINE_STORE_U64,
PLUGIN_N_CB_SUBTYPES,
};
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 4fc6c3739b2..c5cac897a0b 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -305,12 +305,12 @@ void qemu_plugin_register_vcpu_tb_exec_cb(struct qemu_plugin_tb *tb,
* enum qemu_plugin_op - describes an inline op
*
* @QEMU_PLUGIN_INLINE_ADD_U64: add an immediate value uint64_t
- *
- * Note: currently only a single inline op is supported.
+ * @QEMU_PLUGIN_INLINE_STORE_U64: store an immediate value uint64_t
*/
enum qemu_plugin_op {
QEMU_PLUGIN_INLINE_ADD_U64,
+ QEMU_PLUGIN_INLINE_STORE_U64,
};
/**
diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index 494467e0833..02c894106e2 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -46,8 +46,9 @@
#include "qemu/plugin.h"
#include "cpu.h"
#include "tcg/tcg.h"
-#include "tcg/tcg-temp-internal.h"
+#include "tcg/tcg-internal.h"
#include "tcg/tcg-op.h"
+#include "tcg/tcg-temp-internal.h"
#include "exec/exec-all.h"
#include "exec/plugin-gen.h"
#include "exec/translator.h"
@@ -82,6 +83,7 @@ enum plugin_gen_cb {
PLUGIN_GEN_CB_UDATA,
PLUGIN_GEN_CB_UDATA_R,
PLUGIN_GEN_CB_INLINE_ADD_U64,
+ PLUGIN_GEN_CB_INLINE_STORE_U64,
PLUGIN_GEN_CB_MEM,
PLUGIN_GEN_ENABLE_MEM_HELPER,
PLUGIN_GEN_DISABLE_MEM_HELPER,
@@ -153,6 +155,30 @@ static void gen_empty_inline_cb_add_u64(void)
tcg_temp_free_i32(cpu_index);
}
+static void gen_empty_inline_cb_store_u64(void)
+{
+ TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
+ TCGv_ptr cpu_index_as_ptr = tcg_temp_ebb_new_ptr();
+ TCGv_i64 val = tcg_temp_ebb_new_i64();
+ TCGv_ptr ptr = tcg_temp_ebb_new_ptr();
+
+ tcg_gen_ld_i32(cpu_index, tcg_env,
+ -offsetof(ArchCPU, env) + offsetof(CPUState, cpu_index));
+ /* second operand will be replaced by immediate value */
+ tcg_gen_mul_i32(cpu_index, cpu_index, cpu_index);
+ tcg_gen_ext_i32_ptr(cpu_index_as_ptr, cpu_index);
+ tcg_gen_movi_ptr(ptr, 0);
+ tcg_gen_add_ptr(ptr, ptr, cpu_index_as_ptr);
+
+ tcg_gen_movi_i64(val, 0);
+ tcg_gen_st_i64(val, ptr, 0);
+
+ tcg_temp_free_ptr(ptr);
+ tcg_temp_free_i64(val);
+ tcg_temp_free_ptr(cpu_index_as_ptr);
+ tcg_temp_free_i32(cpu_index);
+}
+
static void gen_empty_mem_cb(TCGv_i64 addr, uint32_t info)
{
TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
@@ -218,6 +244,8 @@ static void plugin_gen_empty_callback(enum plugin_gen_from from)
/* emit inline op before any callback */
gen_wrapped(from, PLUGIN_GEN_CB_INLINE_ADD_U64,
gen_empty_inline_cb_add_u64);
+ gen_wrapped(from, PLUGIN_GEN_CB_INLINE_STORE_U64,
+ gen_empty_inline_cb_store_u64);
gen_wrapped(from, PLUGIN_GEN_CB_UDATA, gen_empty_udata_cb_no_rwg);
gen_wrapped(from, PLUGIN_GEN_CB_UDATA_R, gen_empty_udata_cb_no_wg);
break;
@@ -235,6 +263,11 @@ void plugin_gen_empty_mem_callback(TCGv_i64 addr, uint32_t info)
gen_empty_inline_cb_add_u64();
tcg_gen_plugin_cb_end();
+ gen_plugin_cb_start(PLUGIN_GEN_FROM_MEM,
+ PLUGIN_GEN_CB_INLINE_STORE_U64, rw);
+ gen_empty_inline_cb_store_u64();
+ tcg_gen_plugin_cb_end();
+
gen_plugin_cb_start(PLUGIN_GEN_FROM_MEM, PLUGIN_GEN_CB_MEM, rw);
gen_empty_mem_cb(addr, info);
tcg_gen_plugin_cb_end();
@@ -352,6 +385,20 @@ static TCGOp *copy_st_i64(TCGOp **begin_op, TCGOp *op)
return op;
}
+static TCGOp *copy_mov_i64(TCGOp **begin_op, TCGOp *op, uint64_t v)
+{
+ if (TCG_TARGET_REG_BITS == 32) {
+ op = copy_op(begin_op, op, INDEX_op_mov_i32);
+ op->args[1] = tcgv_i32_arg(TCGV_LOW(tcg_constant_i64(v)));
+ op = copy_op(begin_op, op, INDEX_op_mov_i32);
+ op->args[1] = tcgv_i32_arg(TCGV_HIGH(tcg_constant_i64(v)));
+ } else {
+ op = copy_op(begin_op, op, INDEX_op_mov_i64);
+ op->args[1] = tcgv_i64_arg(tcg_constant_i64(v));
+ }
+ return op;
+}
+
static TCGOp *copy_add_i64(TCGOp **begin_op, TCGOp *op, uint64_t v)
{
if (TCG_TARGET_REG_BITS == 32) {
@@ -455,6 +502,24 @@ static TCGOp *append_inline_cb_add_u64(const struct qemu_plugin_dyn_cb *cb,
return op;
}
+static TCGOp *append_inline_cb_store_u64(const struct qemu_plugin_dyn_cb *cb,
+ TCGOp *begin_op, TCGOp *op,
+ int *unused)
+{
+ char *ptr = cb->inline_insn.entry.score->data->data;
+ size_t elem_size = g_array_get_element_size(
+ cb->inline_insn.entry.score->data);
+ size_t offset = cb->inline_insn.entry.offset;
+ op = copy_ld_i32(&begin_op, op);
+ op = copy_mul_i32(&begin_op, op, elem_size);
+ op = copy_ext_i32_ptr(&begin_op, op);
+ op = copy_const_ptr(&begin_op, op, ptr + offset);
+ op = copy_add_ptr(&begin_op, op);
+ op = copy_mov_i64(&begin_op, op, cb->inline_insn.imm);
+ op = copy_st_i64(&begin_op, op);
+ return op;
+}
+
static TCGOp *append_mem_cb(const struct qemu_plugin_dyn_cb *cb,
TCGOp *begin_op, TCGOp *op, int *cb_idx)
{
@@ -542,6 +607,12 @@ inject_inline_cb_add_u64(const GArray *cbs, TCGOp *begin_op, op_ok_fn ok)
inject_cb_type(cbs, begin_op, append_inline_cb_add_u64, ok);
}
+static void
+inject_inline_cb_store_u64(const GArray *cbs, TCGOp *begin_op, op_ok_fn ok)
+{
+ inject_cb_type(cbs, begin_op, append_inline_cb_store_u64, ok);
+}
+
static void
inject_mem_cb(const GArray *cbs, TCGOp *begin_op)
{
@@ -583,13 +654,14 @@ static void inject_mem_enable_helper(struct qemu_plugin_tb *ptb,
struct qemu_plugin_insn *plugin_insn,
TCGOp *begin_op)
{
- GArray *cbs[2];
+ GArray *cbs[3];
GArray *arr;
size_t n_cbs, i;
/* emit inline op before any callback */
cbs[0] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_INLINE_ADD_U64];
- cbs[1] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_REGULAR];
+ cbs[1] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_INLINE_STORE_U64];
+ cbs[2] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_REGULAR];
n_cbs = 0;
for (i = 0; i < ARRAY_SIZE(cbs); i++) {
@@ -662,6 +734,13 @@ static void plugin_gen_tb_inline_add_u64(const struct qemu_plugin_tb *ptb,
begin_op, op_ok);
}
+static void plugin_gen_tb_inline_store_u64(const struct qemu_plugin_tb *ptb,
+ TCGOp *begin_op)
+{
+ inject_inline_cb_store_u64(ptb->cbs[PLUGIN_CB_INLINE_STORE_U64],
+ begin_op, op_ok);
+}
+
static void plugin_gen_insn_udata(const struct qemu_plugin_tb *ptb,
TCGOp *begin_op, int insn_idx)
{
@@ -686,6 +765,14 @@ static void plugin_gen_insn_inline_add_u64(const struct qemu_plugin_tb *ptb,
insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_INLINE_ADD_U64], begin_op, op_ok);
}
+static void plugin_gen_insn_inline_store_u64(const struct qemu_plugin_tb *ptb,
+ TCGOp *begin_op, int insn_idx)
+{
+ struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
+ inject_inline_cb_store_u64(
+ insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_INLINE_STORE_U64], begin_op, op_ok);
+}
+
static void plugin_gen_mem_regular(const struct qemu_plugin_tb *ptb,
TCGOp *begin_op, int insn_idx)
{
@@ -701,6 +788,15 @@ static void plugin_gen_mem_inline_add_u64(const struct qemu_plugin_tb *ptb,
begin_op, op_rw);
}
+static void plugin_gen_mem_inline_store_u64(const struct qemu_plugin_tb *ptb,
+ TCGOp *begin_op, int insn_idx)
+{
+ struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
+ inject_inline_cb_store_u64(
+ insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_INLINE_STORE_U64],
+ begin_op, op_rw);
+}
+
static void plugin_gen_enable_mem_helper(struct qemu_plugin_tb *ptb,
TCGOp *begin_op, int insn_idx)
{
@@ -750,6 +846,9 @@ static void pr_ops(void)
case PLUGIN_GEN_CB_INLINE_ADD_U64:
type = "inline add u64";
break;
+ case PLUGIN_GEN_CB_INLINE_STORE_U64:
+ type = "inline store u64";
+ break;
case PLUGIN_GEN_CB_MEM:
type = "mem";
break;
@@ -801,6 +900,9 @@ static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
case PLUGIN_GEN_CB_INLINE_ADD_U64:
plugin_gen_tb_inline_add_u64(plugin_tb, op);
break;
+ case PLUGIN_GEN_CB_INLINE_STORE_U64:
+ plugin_gen_tb_inline_store_u64(plugin_tb, op);
+ break;
default:
g_assert_not_reached();
}
@@ -820,6 +922,9 @@ static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
case PLUGIN_GEN_CB_INLINE_ADD_U64:
plugin_gen_insn_inline_add_u64(plugin_tb, op, insn_idx);
break;
+ case PLUGIN_GEN_CB_INLINE_STORE_U64:
+ plugin_gen_insn_inline_store_u64(plugin_tb, op, insn_idx);
+ break;
case PLUGIN_GEN_ENABLE_MEM_HELPER:
plugin_gen_enable_mem_helper(plugin_tb, op, insn_idx);
break;
@@ -839,6 +944,9 @@ static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
case PLUGIN_GEN_CB_INLINE_ADD_U64:
plugin_gen_mem_inline_add_u64(plugin_tb, op, insn_idx);
break;
+ case PLUGIN_GEN_CB_INLINE_STORE_U64:
+ plugin_gen_mem_inline_store_u64(plugin_tb, op, insn_idx);
+ break;
default:
g_assert_not_reached();
}
diff --git a/plugins/api.c b/plugins/api.c
index e3bcacfa99a..4a033c16f83 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -60,6 +60,8 @@ static enum plugin_dyn_cb_subtype op_to_cb_subtype(enum qemu_plugin_op op)
switch (op) {
case QEMU_PLUGIN_INLINE_ADD_U64:
return PLUGIN_CB_INLINE_ADD_U64;
+ case QEMU_PLUGIN_INLINE_STORE_U64:
+ return PLUGIN_CB_INLINE_STORE_U64;
default:
g_assert_not_reached();
}
diff --git a/plugins/core.c b/plugins/core.c
index a641a366ef9..11f72594229 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -489,6 +489,9 @@ void exec_inline_op(struct qemu_plugin_dyn_cb *cb, int cpu_index)
case QEMU_PLUGIN_INLINE_ADD_U64:
*val += cb->inline_insn.imm;
break;
+ case QEMU_PLUGIN_INLINE_STORE_U64:
+ *val = cb->inline_insn.imm;
+ break;
default:
g_assert_not_reached();
}
@@ -516,6 +519,7 @@ void qemu_plugin_vcpu_mem_cb(CPUState *cpu, uint64_t vaddr,
vaddr, cb->userp);
break;
case PLUGIN_CB_INLINE_ADD_U64:
+ case PLUGIN_CB_INLINE_STORE_U64:
exec_inline_op(cb, cpu->cpu_index);
break;
default:
--
2.43.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/5] tests/plugin/inline: add test for STORE_U64 inline op
2024-02-29 5:53 [PATCH 0/5] TCG plugins new inline operations Pierrick Bouvier
2024-02-29 5:53 ` [PATCH 1/5] plugins: prepare introduction of new inline ops Pierrick Bouvier
2024-02-29 5:53 ` [PATCH 2/5] plugins: add new inline op STORE_U64 Pierrick Bouvier
@ 2024-02-29 5:53 ` Pierrick Bouvier
2024-02-29 5:53 ` [PATCH 4/5] plugins: conditional callbacks Pierrick Bouvier
` (2 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Pierrick Bouvier @ 2024-02-29 5:53 UTC (permalink / raw)
To: qemu-devel
Cc: Mahmoud Mandour, Paolo Bonzini, Alexandre Iooss, Alex Bennée,
Pierrick Bouvier, Richard Henderson
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
tests/plugin/inline.c | 41 +++++++++++++++++++++++++++++++++++++----
1 file changed, 37 insertions(+), 4 deletions(-)
diff --git a/tests/plugin/inline.c b/tests/plugin/inline.c
index 0163e9b51c5..30acc7a1838 100644
--- a/tests/plugin/inline.c
+++ b/tests/plugin/inline.c
@@ -22,6 +22,12 @@ typedef struct {
uint64_t count_mem_inline;
} CPUCount;
+typedef struct {
+ uint64_t data_insn;
+ uint64_t data_tb;
+ uint64_t data_mem;
+} CPUData;
+
static struct qemu_plugin_scoreboard *counts;
static qemu_plugin_u64 count_tb;
static qemu_plugin_u64 count_tb_inline;
@@ -29,6 +35,10 @@ static qemu_plugin_u64 count_insn;
static qemu_plugin_u64 count_insn_inline;
static qemu_plugin_u64 count_mem;
static qemu_plugin_u64 count_mem_inline;
+static struct qemu_plugin_scoreboard *data;
+static qemu_plugin_u64 data_insn;
+static qemu_plugin_u64 data_tb;
+static qemu_plugin_u64 data_mem;
static uint64_t global_count_tb;
static uint64_t global_count_insn;
@@ -109,11 +119,13 @@ static void plugin_exit(qemu_plugin_id_t id, void *udata)
stats_mem();
qemu_plugin_scoreboard_free(counts);
+ qemu_plugin_scoreboard_free(data);
}
static void vcpu_tb_exec(unsigned int cpu_index, void *udata)
{
qemu_plugin_u64_add(count_tb, cpu_index, 1);
+ g_assert(qemu_plugin_u64_get(data_tb, cpu_index) == (uintptr_t) udata);
g_mutex_lock(&tb_lock);
max_cpu_index = MAX(max_cpu_index, cpu_index);
global_count_tb++;
@@ -123,6 +135,7 @@ static void vcpu_tb_exec(unsigned int cpu_index, void *udata)
static void vcpu_insn_exec(unsigned int cpu_index, void *udata)
{
qemu_plugin_u64_add(count_insn, cpu_index, 1);
+ g_assert(qemu_plugin_u64_get(data_insn, cpu_index) == (uintptr_t) udata);
g_mutex_lock(&insn_lock);
global_count_insn++;
g_mutex_unlock(&insn_lock);
@@ -131,9 +144,10 @@ static void vcpu_insn_exec(unsigned int cpu_index, void *udata)
static void vcpu_mem_access(unsigned int cpu_index,
qemu_plugin_meminfo_t info,
uint64_t vaddr,
- void *userdata)
+ void *udata)
{
qemu_plugin_u64_add(count_mem, cpu_index, 1);
+ g_assert(qemu_plugin_u64_get(data_mem, cpu_index) == (uintptr_t) udata);
g_mutex_lock(&mem_lock);
global_count_mem++;
g_mutex_unlock(&mem_lock);
@@ -141,20 +155,34 @@ static void vcpu_mem_access(unsigned int cpu_index,
static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
{
+ void *tb_store = tb;
qemu_plugin_register_vcpu_tb_exec_cb(
- tb, vcpu_tb_exec, QEMU_PLUGIN_CB_NO_REGS, 0);
+ tb, vcpu_tb_exec, QEMU_PLUGIN_CB_NO_REGS, tb_store);
qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
tb, QEMU_PLUGIN_INLINE_ADD_U64, count_tb_inline, 1);
+ qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
+ tb, QEMU_PLUGIN_INLINE_STORE_U64, data_tb, (uintptr_t) tb_store);
for (int idx = 0; idx < qemu_plugin_tb_n_insns(tb); ++idx) {
struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, idx);
+ void *insn_store = insn;
+ void *mem_store = (char *)insn_store + 0xff;
+
qemu_plugin_register_vcpu_insn_exec_cb(
- insn, vcpu_insn_exec, QEMU_PLUGIN_CB_NO_REGS, 0);
+ insn, vcpu_insn_exec, QEMU_PLUGIN_CB_NO_REGS, insn_store);
+ qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
+ insn, QEMU_PLUGIN_INLINE_STORE_U64, data_insn,
+ (uintptr_t) insn_store);
qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
insn, QEMU_PLUGIN_INLINE_ADD_U64, count_insn_inline, 1);
+
qemu_plugin_register_vcpu_mem_cb(insn, &vcpu_mem_access,
QEMU_PLUGIN_CB_NO_REGS,
- QEMU_PLUGIN_MEM_RW, 0);
+ QEMU_PLUGIN_MEM_RW, mem_store);
+ qemu_plugin_register_vcpu_mem_inline_per_vcpu(
+ insn, QEMU_PLUGIN_MEM_RW,
+ QEMU_PLUGIN_INLINE_STORE_U64,
+ data_mem, (uintptr_t) mem_store);
qemu_plugin_register_vcpu_mem_inline_per_vcpu(
insn, QEMU_PLUGIN_MEM_RW,
QEMU_PLUGIN_INLINE_ADD_U64,
@@ -179,6 +207,11 @@ int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info,
counts, CPUCount, count_insn_inline);
count_mem_inline = qemu_plugin_scoreboard_u64_in_struct(
counts, CPUCount, count_mem_inline);
+ data = qemu_plugin_scoreboard_new(sizeof(CPUData));
+ data_insn = qemu_plugin_scoreboard_u64_in_struct(data, CPUData, data_insn);
+ data_tb = qemu_plugin_scoreboard_u64_in_struct(data, CPUData, data_tb);
+ data_mem = qemu_plugin_scoreboard_u64_in_struct(data, CPUData, data_mem);
+
qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
--
2.43.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 4/5] plugins: conditional callbacks
2024-02-29 5:53 [PATCH 0/5] TCG plugins new inline operations Pierrick Bouvier
` (2 preceding siblings ...)
2024-02-29 5:53 ` [PATCH 3/5] tests/plugin/inline: add test for STORE_U64 inline op Pierrick Bouvier
@ 2024-02-29 5:53 ` Pierrick Bouvier
2024-03-11 10:08 ` Alex Bennée
2024-03-11 15:43 ` Alex Bennée
2024-02-29 5:53 ` [PATCH 5/5] tests/plugin/inline: add test for condition callback Pierrick Bouvier
2024-03-08 10:41 ` [PATCH 0/5] TCG plugins new inline operations Pierrick Bouvier
5 siblings, 2 replies; 14+ messages in thread
From: Pierrick Bouvier @ 2024-02-29 5:53 UTC (permalink / raw)
To: qemu-devel
Cc: Mahmoud Mandour, Paolo Bonzini, Alexandre Iooss, Alex Bennée,
Pierrick Bouvier, Richard Henderson
Extend plugins API to support callback called with a given criteria
(evaluated inline).
Added functions:
- qemu_plugin_register_vcpu_tb_exec_cond_cb
- qemu_plugin_register_vcpu_insn_exec_cond_cb
They expect as parameter a condition, a qemu_plugin_u64_t (op1) and an
immediate (op2). Callback is called if op1 |cond| op2 is true.
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
include/qemu/plugin.h | 7 ++
include/qemu/qemu-plugin.h | 76 +++++++++++++++
plugins/plugin.h | 8 ++
accel/tcg/plugin-gen.c | 174 ++++++++++++++++++++++++++++++++++-
plugins/api.c | 51 ++++++++++
plugins/core.c | 19 ++++
plugins/qemu-plugins.symbols | 2 +
7 files changed, 334 insertions(+), 3 deletions(-)
diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index d92d64744e6..056102b2361 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -74,6 +74,8 @@ enum plugin_dyn_cb_type {
enum plugin_dyn_cb_subtype {
PLUGIN_CB_REGULAR,
PLUGIN_CB_REGULAR_R,
+ PLUGIN_CB_COND,
+ PLUGIN_CB_COND_R,
PLUGIN_CB_INLINE_ADD_U64,
PLUGIN_CB_INLINE_STORE_U64,
PLUGIN_N_CB_SUBTYPES,
@@ -97,6 +99,11 @@ struct qemu_plugin_dyn_cb {
enum qemu_plugin_op op;
uint64_t imm;
} inline_insn;
+ struct {
+ qemu_plugin_u64 entry;
+ enum qemu_plugin_cond cond;
+ uint64_t imm;
+ } cond_cb;
};
};
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index c5cac897a0b..337de25ece7 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -262,6 +262,29 @@ enum qemu_plugin_mem_rw {
QEMU_PLUGIN_MEM_RW,
};
+/**
+ * enum qemu_plugin_cond - condition to enable callback
+ *
+ * @QEMU_PLUGIN_COND_NEVER: false
+ * @QEMU_PLUGIN_COND_ALWAYS: true
+ * @QEMU_PLUGIN_COND_EQ: is equal?
+ * @QEMU_PLUGIN_COND_NE: is not equal?
+ * @QEMU_PLUGIN_COND_LT: is less than?
+ * @QEMU_PLUGIN_COND_LE: is less than or equal?
+ * @QEMU_PLUGIN_COND_GT: is greater than?
+ * @QEMU_PLUGIN_COND_GE: is greater than or equal?
+ */
+enum qemu_plugin_cond {
+ QEMU_PLUGIN_COND_NEVER,
+ QEMU_PLUGIN_COND_ALWAYS,
+ QEMU_PLUGIN_COND_EQ,
+ QEMU_PLUGIN_COND_NE,
+ QEMU_PLUGIN_COND_LT,
+ QEMU_PLUGIN_COND_LE,
+ QEMU_PLUGIN_COND_GT,
+ QEMU_PLUGIN_COND_GE,
+};
+
/**
* typedef qemu_plugin_vcpu_tb_trans_cb_t - translation callback
* @id: unique plugin id
@@ -301,6 +324,32 @@ void qemu_plugin_register_vcpu_tb_exec_cb(struct qemu_plugin_tb *tb,
enum qemu_plugin_cb_flags flags,
void *userdata);
+/**
+ * qemu_plugin_register_vcpu_tb_exec_cond_cb() - register conditional callback
+ * @tb: the opaque qemu_plugin_tb handle for the translation
+ * @cb: callback function
+ * @cond: condition to enable callback
+ * @entry: first operand for condition
+ * @imm: second operand for condition
+ * @flags: does the plugin read or write the CPU's registers?
+ * @userdata: any plugin data to pass to the @cb?
+ *
+ * The @cb function is called when a translated unit executes if
+ * entry @cond imm is true.
+ * If condition is QEMU_PLUGIN_COND_ALWAYS, condition is never interpreted and
+ * this function is equivalent to qemu_plugin_register_vcpu_tb_exec_cb.
+ * If condition QEMU_PLUGIN_COND_NEVER, condition is never interpreted and
+ * callback is never installed.
+ */
+QEMU_PLUGIN_API
+void qemu_plugin_register_vcpu_tb_exec_cond_cb(struct qemu_plugin_tb *tb,
+ qemu_plugin_vcpu_udata_cb_t cb,
+ enum qemu_plugin_cb_flags flags,
+ enum qemu_plugin_cond cond,
+ qemu_plugin_u64 entry,
+ uint64_t imm,
+ void *userdata);
+
/**
* enum qemu_plugin_op - describes an inline op
*
@@ -344,6 +393,33 @@ void qemu_plugin_register_vcpu_insn_exec_cb(struct qemu_plugin_insn *insn,
enum qemu_plugin_cb_flags flags,
void *userdata);
+/**
+ * qemu_plugin_register_vcpu_insn_exec_cond_cb() - conditional insn execution cb
+ * @insn: the opaque qemu_plugin_insn handle for an instruction
+ * @cb: callback function
+ * @flags: does the plugin read or write the CPU's registers?
+ * @cond: condition to enable callback
+ * @entry: first operand for condition
+ * @imm: second operand for condition
+ * @userdata: any plugin data to pass to the @cb?
+ *
+ * The @cb function is called when an instruction executes if
+ * entry @cond imm is true.
+ * If condition is QEMU_PLUGIN_COND_ALWAYS, condition is never interpreted and
+ * this function is equivalent to qemu_plugin_register_vcpu_insn_exec_cb.
+ * If condition QEMU_PLUGIN_COND_NEVER, condition is never interpreted and
+ * callback is never installed.
+ */
+QEMU_PLUGIN_API
+void qemu_plugin_register_vcpu_insn_exec_cond_cb(
+ struct qemu_plugin_insn *insn,
+ qemu_plugin_vcpu_udata_cb_t cb,
+ enum qemu_plugin_cb_flags flags,
+ enum qemu_plugin_cond cond,
+ qemu_plugin_u64 entry,
+ uint64_t imm,
+ void *userdata);
+
/**
* qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu() - insn exec inline op
* @insn: the opaque qemu_plugin_insn handle for an instruction
diff --git a/plugins/plugin.h b/plugins/plugin.h
index 696b1fa38b0..118cc99bb2a 100644
--- a/plugins/plugin.h
+++ b/plugins/plugin.h
@@ -94,6 +94,14 @@ plugin_register_dyn_cb__udata(GArray **arr,
qemu_plugin_vcpu_udata_cb_t cb,
enum qemu_plugin_cb_flags flags, void *udata);
+void
+plugin_register_dyn_cond_cb__udata(GArray **arr,
+ qemu_plugin_vcpu_udata_cb_t cb,
+ enum qemu_plugin_cb_flags flags,
+ enum qemu_plugin_cond cond,
+ qemu_plugin_u64 entry,
+ uint64_t imm,
+ void *udata);
void plugin_register_vcpu_mem_cb(GArray **arr,
void *cb,
diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index 02c894106e2..15762a26439 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -46,6 +46,7 @@
#include "qemu/plugin.h"
#include "cpu.h"
#include "tcg/tcg.h"
+#include "tcg/tcg-cond.h"
#include "tcg/tcg-internal.h"
#include "tcg/tcg-op.h"
#include "tcg/tcg-temp-internal.h"
@@ -82,6 +83,8 @@ enum plugin_gen_from {
enum plugin_gen_cb {
PLUGIN_GEN_CB_UDATA,
PLUGIN_GEN_CB_UDATA_R,
+ PLUGIN_GEN_CB_COND_UDATA,
+ PLUGIN_GEN_CB_COND_UDATA_R,
PLUGIN_GEN_CB_INLINE_ADD_U64,
PLUGIN_GEN_CB_INLINE_STORE_U64,
PLUGIN_GEN_CB_MEM,
@@ -119,16 +122,58 @@ static void gen_empty_udata_cb(void (*gen_helper)(TCGv_i32, TCGv_ptr))
tcg_temp_free_i32(cpu_index);
}
+static void gen_empty_cond_udata_cb(void (*gen_helper)(TCGv_i32, TCGv_ptr))
+{
+ TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
+ TCGv_i32 cpu_offset = tcg_temp_ebb_new_i32();
+ TCGv_ptr cpu_index_as_ptr = tcg_temp_ebb_new_ptr();
+ TCGv_i64 val = tcg_temp_ebb_new_i64();
+ TCGv_ptr ptr = tcg_temp_ebb_new_ptr();
+ TCGv_ptr udata = tcg_temp_ebb_new_ptr();
+ TCGLabel *after_cb = gen_new_label();
+
+ tcg_gen_movi_ptr(udata, 0);
+ tcg_gen_ld_i32(cpu_index, tcg_env,
+ -offsetof(ArchCPU, env) + offsetof(CPUState, cpu_index));
+ /* second operand will be replaced by immediate value */
+ tcg_gen_mul_i32(cpu_offset, cpu_index, cpu_index);
+ tcg_gen_ext_i32_ptr(cpu_index_as_ptr, cpu_offset);
+ tcg_gen_movi_ptr(ptr, 0);
+ tcg_gen_add_ptr(ptr, ptr, cpu_index_as_ptr);
+ tcg_gen_ld_i64(val, ptr, 0);
+
+ tcg_gen_brcondi_i64(TCG_COND_EQ, val, 0, after_cb);
+ gen_helper(cpu_index, udata);
+ gen_set_label(after_cb);
+
+ tcg_temp_free_ptr(udata);
+ tcg_temp_free_ptr(ptr);
+ tcg_temp_free_i64(val);
+ tcg_temp_free_ptr(cpu_index_as_ptr);
+ tcg_temp_free_i32(cpu_offset);
+ tcg_temp_free_i32(cpu_index);
+}
+
static void gen_empty_udata_cb_no_wg(void)
{
gen_empty_udata_cb(gen_helper_plugin_vcpu_udata_cb_no_wg);
}
+static void gen_empty_cond_udata_cb_no_wg(void)
+{
+ gen_empty_cond_udata_cb(gen_helper_plugin_vcpu_udata_cb_no_wg);
+}
+
static void gen_empty_udata_cb_no_rwg(void)
{
gen_empty_udata_cb(gen_helper_plugin_vcpu_udata_cb_no_rwg);
}
+static void gen_empty_cond_udata_cb_no_rwg(void)
+{
+ gen_empty_cond_udata_cb(gen_helper_plugin_vcpu_udata_cb_no_rwg);
+}
+
static void gen_empty_inline_cb_add_u64(void)
{
TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
@@ -248,6 +293,10 @@ static void plugin_gen_empty_callback(enum plugin_gen_from from)
gen_empty_inline_cb_store_u64);
gen_wrapped(from, PLUGIN_GEN_CB_UDATA, gen_empty_udata_cb_no_rwg);
gen_wrapped(from, PLUGIN_GEN_CB_UDATA_R, gen_empty_udata_cb_no_wg);
+ gen_wrapped(from, PLUGIN_GEN_CB_COND_UDATA,
+ gen_empty_cond_udata_cb_no_rwg);
+ gen_wrapped(from, PLUGIN_GEN_CB_COND_UDATA_R,
+ gen_empty_cond_udata_cb_no_wg);
break;
default:
g_assert_not_reached();
@@ -456,6 +505,29 @@ static TCGOp *copy_call(TCGOp **begin_op, TCGOp *op, void *func, int *cb_idx)
return op;
}
+static TCGOp *copy_brcondi_i64(TCGOp **begin_op, TCGOp *op, TCGCond cond,
+ uint64_t imm)
+{
+ if (TCG_TARGET_REG_BITS == 32) {
+ op = copy_op(begin_op, op, INDEX_op_brcond2_i32);
+ /* low(arg1), high(arg1), 32(arg2), 32(arg2 >> 32), cond, label */
+ op->args[2] = tcgv_i32_arg(tcg_constant_i32(imm));
+ op->args[3] = tcgv_i32_arg(tcg_constant_i32(imm >> 32));
+ op->args[4] = cond;
+ } else {
+ op = copy_op(begin_op, op, INDEX_op_brcond_i64);
+ /* arg1, arg2, cond, label */
+ op->args[1] = tcgv_i64_arg(tcg_constant_i64(imm));
+ op->args[2] = cond;
+ }
+ return op;
+}
+
+static TCGOp *copy_set_label(TCGOp **begin_op, TCGOp *op)
+{
+ return copy_op(begin_op, op, INDEX_op_set_label);
+}
+
/*
* When we append/replace ops here we are sensitive to changing patterns of
* TCGOps generated by the tcg_gen_FOO calls when we generated the
@@ -482,6 +554,50 @@ static TCGOp *append_udata_cb(const struct qemu_plugin_dyn_cb *cb,
return op;
}
+static TCGCond plugin_cond_to_tcgcond(enum qemu_plugin_cond cond)
+{
+ switch (cond) {
+ case QEMU_PLUGIN_COND_EQ:
+ return TCG_COND_EQ;
+ case QEMU_PLUGIN_COND_NE:
+ return TCG_COND_NE;
+ case QEMU_PLUGIN_COND_LT:
+ return TCG_COND_LTU;
+ case QEMU_PLUGIN_COND_LE:
+ return TCG_COND_LEU;
+ case QEMU_PLUGIN_COND_GT:
+ return TCG_COND_GTU;
+ case QEMU_PLUGIN_COND_GE:
+ return TCG_COND_GEU;
+ default:
+ /* ALWAYS and NEVER conditions should never reach */
+ g_assert_not_reached();
+ }
+}
+
+static TCGOp *append_cond_udata_cb(const struct qemu_plugin_dyn_cb *cb,
+ TCGOp *begin_op, TCGOp *op, int *cb_idx)
+{
+ char *ptr = cb->cond_cb.entry.score->data->data;
+ size_t elem_size = g_array_get_element_size(
+ cb->cond_cb.entry.score->data);
+ size_t offset = cb->cond_cb.entry.offset;
+ /* Condition should be negated, as calling the cb is the "else" path */
+ TCGCond cond = tcg_invert_cond(plugin_cond_to_tcgcond(cb->cond_cb.cond));
+
+ op = copy_const_ptr(&begin_op, op, ptr);
+ op = copy_ld_i32(&begin_op, op);
+ op = copy_mul_i32(&begin_op, op, elem_size);
+ op = copy_ext_i32_ptr(&begin_op, op);
+ op = copy_const_ptr(&begin_op, op, ptr + offset);
+ op = copy_add_ptr(&begin_op, op);
+ op = copy_ld_i64(&begin_op, op);
+ op = copy_brcondi_i64(&begin_op, op, cond, cb->cond_cb.imm);
+ op = copy_call(&begin_op, op, cb->f.vcpu_udata, cb_idx);
+ op = copy_set_label(&begin_op, op);
+ return op;
+}
+
static TCGOp *append_inline_cb_add_u64(const struct qemu_plugin_dyn_cb *cb,
TCGOp *begin_op, TCGOp *op,
int *unused)
@@ -601,6 +717,12 @@ inject_udata_cb(const GArray *cbs, TCGOp *begin_op)
inject_cb_type(cbs, begin_op, append_udata_cb, op_ok);
}
+static void
+inject_cond_udata_cb(const GArray *cbs, TCGOp *begin_op)
+{
+ inject_cb_type(cbs, begin_op, append_cond_udata_cb, op_ok);
+}
+
static void
inject_inline_cb_add_u64(const GArray *cbs, TCGOp *begin_op, op_ok_fn ok)
{
@@ -654,7 +776,7 @@ static void inject_mem_enable_helper(struct qemu_plugin_tb *ptb,
struct qemu_plugin_insn *plugin_insn,
TCGOp *begin_op)
{
- GArray *cbs[3];
+ GArray *cbs[4];
GArray *arr;
size_t n_cbs, i;
@@ -662,6 +784,7 @@ static void inject_mem_enable_helper(struct qemu_plugin_tb *ptb,
cbs[0] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_INLINE_ADD_U64];
cbs[1] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_INLINE_STORE_U64];
cbs[2] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_REGULAR];
+ cbs[3] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_COND];
n_cbs = 0;
for (i = 0; i < ARRAY_SIZE(cbs); i++) {
@@ -727,6 +850,18 @@ static void plugin_gen_tb_udata_r(const struct qemu_plugin_tb *ptb,
inject_udata_cb(ptb->cbs[PLUGIN_CB_REGULAR_R], begin_op);
}
+static void plugin_gen_tb_cond_udata(const struct qemu_plugin_tb *ptb,
+ TCGOp *begin_op)
+{
+ inject_cond_udata_cb(ptb->cbs[PLUGIN_CB_COND], begin_op);
+}
+
+static void plugin_gen_tb_cond_udata_r(const struct qemu_plugin_tb *ptb,
+ TCGOp *begin_op)
+{
+ inject_cond_udata_cb(ptb->cbs[PLUGIN_CB_COND_R], begin_op);
+}
+
static void plugin_gen_tb_inline_add_u64(const struct qemu_plugin_tb *ptb,
TCGOp *begin_op)
{
@@ -745,7 +880,6 @@ static void plugin_gen_insn_udata(const struct qemu_plugin_tb *ptb,
TCGOp *begin_op, int insn_idx)
{
struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
-
inject_udata_cb(insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_REGULAR], begin_op);
}
@@ -753,10 +887,23 @@ static void plugin_gen_insn_udata_r(const struct qemu_plugin_tb *ptb,
TCGOp *begin_op, int insn_idx)
{
struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
-
inject_udata_cb(insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_REGULAR_R], begin_op);
}
+static void plugin_gen_insn_cond_udata(const struct qemu_plugin_tb *ptb,
+ TCGOp *begin_op, int insn_idx)
+{
+ struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
+ inject_cond_udata_cb(insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_COND], begin_op);
+}
+
+static void plugin_gen_insn_cond_udata_r(const struct qemu_plugin_tb *ptb,
+ TCGOp *begin_op, int insn_idx)
+{
+ struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
+ inject_cond_udata_cb(insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_COND_R], begin_op);
+}
+
static void plugin_gen_insn_inline_add_u64(const struct qemu_plugin_tb *ptb,
TCGOp *begin_op, int insn_idx)
{
@@ -843,6 +990,15 @@ static void pr_ops(void)
case PLUGIN_GEN_CB_UDATA:
type = "udata";
break;
+ case PLUGIN_GEN_CB_UDATA_R:
+ type = "udata (read)";
+ break;
+ case PLUGIN_GEN_CB_COND_UDATA:
+ type = "cond udata";
+ break;
+ case PLUGIN_GEN_CB_COND_UDATA_R:
+ type = "cond udata (read)";
+ break;
case PLUGIN_GEN_CB_INLINE_ADD_U64:
type = "inline add u64";
break;
@@ -897,6 +1053,12 @@ static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
case PLUGIN_GEN_CB_UDATA_R:
plugin_gen_tb_udata_r(plugin_tb, op);
break;
+ case PLUGIN_GEN_CB_COND_UDATA:
+ plugin_gen_tb_cond_udata(plugin_tb, op);
+ break;
+ case PLUGIN_GEN_CB_COND_UDATA_R:
+ plugin_gen_tb_cond_udata_r(plugin_tb, op);
+ break;
case PLUGIN_GEN_CB_INLINE_ADD_U64:
plugin_gen_tb_inline_add_u64(plugin_tb, op);
break;
@@ -919,6 +1081,12 @@ static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
case PLUGIN_GEN_CB_UDATA_R:
plugin_gen_insn_udata_r(plugin_tb, op, insn_idx);
break;
+ case PLUGIN_GEN_CB_COND_UDATA:
+ plugin_gen_insn_cond_udata(plugin_tb, op, insn_idx);
+ break;
+ case PLUGIN_GEN_CB_COND_UDATA_R:
+ plugin_gen_insn_cond_udata_r(plugin_tb, op, insn_idx);
+ break;
case PLUGIN_GEN_CB_INLINE_ADD_U64:
plugin_gen_insn_inline_add_u64(plugin_tb, op, insn_idx);
break;
diff --git a/plugins/api.c b/plugins/api.c
index 4a033c16f83..f24d68d7a68 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -113,6 +113,31 @@ void qemu_plugin_register_vcpu_tb_exec_cb(struct qemu_plugin_tb *tb,
}
}
+void qemu_plugin_register_vcpu_tb_exec_cond_cb(struct qemu_plugin_tb *tb,
+ qemu_plugin_vcpu_udata_cb_t cb,
+ enum qemu_plugin_cb_flags flags,
+ enum qemu_plugin_cond cond,
+ qemu_plugin_u64 entry,
+ uint64_t imm,
+ void *udata)
+{
+ if (cond == QEMU_PLUGIN_COND_NEVER || tb->mem_only) {
+ return;
+ }
+ if (cond == QEMU_PLUGIN_COND_ALWAYS) {
+ qemu_plugin_register_vcpu_tb_exec_cb(tb, cb, flags, udata);
+ return;
+ }
+ int index = flags == QEMU_PLUGIN_CB_R_REGS ||
+ flags == QEMU_PLUGIN_CB_RW_REGS ?
+ PLUGIN_CB_COND_R : PLUGIN_CB_COND;
+
+ plugin_register_dyn_cond_cb__udata(&tb->cbs[index],
+ cb, flags,
+ cond, entry, imm,
+ udata);
+}
+
void qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
struct qemu_plugin_tb *tb,
enum qemu_plugin_op op,
@@ -141,6 +166,32 @@ void qemu_plugin_register_vcpu_insn_exec_cb(struct qemu_plugin_insn *insn,
}
}
+void qemu_plugin_register_vcpu_insn_exec_cond_cb(
+ struct qemu_plugin_insn *insn,
+ qemu_plugin_vcpu_udata_cb_t cb,
+ enum qemu_plugin_cb_flags flags,
+ enum qemu_plugin_cond cond,
+ qemu_plugin_u64 entry,
+ uint64_t imm,
+ void *udata)
+{
+ if (cond == QEMU_PLUGIN_COND_NEVER || insn->mem_only) {
+ return;
+ }
+ if (cond == QEMU_PLUGIN_COND_ALWAYS) {
+ qemu_plugin_register_vcpu_insn_exec_cb(insn, cb, flags, udata);
+ return;
+ }
+ int index = flags == QEMU_PLUGIN_CB_R_REGS ||
+ flags == QEMU_PLUGIN_CB_RW_REGS ?
+ PLUGIN_CB_COND_R : PLUGIN_CB_COND;
+
+ plugin_register_dyn_cond_cb__udata(&insn->cbs[PLUGIN_CB_INSN][index],
+ cb, flags,
+ cond, entry, imm,
+ udata);
+}
+
void qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
struct qemu_plugin_insn *insn,
enum qemu_plugin_op op,
diff --git a/plugins/core.c b/plugins/core.c
index 11f72594229..82d5f17438e 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -347,6 +347,25 @@ void plugin_register_dyn_cb__udata(GArray **arr,
dyn_cb->type = PLUGIN_CB_REGULAR;
}
+void plugin_register_dyn_cond_cb__udata(GArray **arr,
+ qemu_plugin_vcpu_udata_cb_t cb,
+ enum qemu_plugin_cb_flags flags,
+ enum qemu_plugin_cond cond,
+ qemu_plugin_u64 entry,
+ uint64_t imm,
+ void *udata)
+{
+ struct qemu_plugin_dyn_cb *dyn_cb = plugin_get_dyn_cb(arr);
+
+ dyn_cb->userp = udata;
+ /* Note flags are discarded as unused. */
+ dyn_cb->f.vcpu_udata = cb;
+ dyn_cb->type = PLUGIN_CB_COND;
+ dyn_cb->cond_cb.cond = cond;
+ dyn_cb->cond_cb.entry = entry;
+ dyn_cb->cond_cb.imm = imm;
+}
+
void plugin_register_vcpu_mem_cb(GArray **arr,
void *cb,
enum qemu_plugin_cb_flags flags,
diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols
index a9fac056c7f..aa0a77a319f 100644
--- a/plugins/qemu-plugins.symbols
+++ b/plugins/qemu-plugins.symbols
@@ -27,6 +27,7 @@
qemu_plugin_register_vcpu_idle_cb;
qemu_plugin_register_vcpu_init_cb;
qemu_plugin_register_vcpu_insn_exec_cb;
+ qemu_plugin_register_vcpu_insn_exec_cond_cb;
qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu;
qemu_plugin_register_vcpu_mem_cb;
qemu_plugin_register_vcpu_mem_inline_per_vcpu;
@@ -34,6 +35,7 @@
qemu_plugin_register_vcpu_syscall_cb;
qemu_plugin_register_vcpu_syscall_ret_cb;
qemu_plugin_register_vcpu_tb_exec_cb;
+ qemu_plugin_register_vcpu_tb_exec_cond_cb;
qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu;
qemu_plugin_register_vcpu_tb_trans_cb;
qemu_plugin_reset;
--
2.43.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 5/5] tests/plugin/inline: add test for condition callback
2024-02-29 5:53 [PATCH 0/5] TCG plugins new inline operations Pierrick Bouvier
` (3 preceding siblings ...)
2024-02-29 5:53 ` [PATCH 4/5] plugins: conditional callbacks Pierrick Bouvier
@ 2024-02-29 5:53 ` Pierrick Bouvier
2024-03-08 10:41 ` [PATCH 0/5] TCG plugins new inline operations Pierrick Bouvier
5 siblings, 0 replies; 14+ messages in thread
From: Pierrick Bouvier @ 2024-02-29 5:53 UTC (permalink / raw)
To: qemu-devel
Cc: Mahmoud Mandour, Paolo Bonzini, Alexandre Iooss, Alex Bennée,
Pierrick Bouvier, Richard Henderson
Count number of tb and insn executed using a conditional callback. We
ensure the callback has been called expected number of time (per vcpu).
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
tests/plugin/inline.c | 87 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 84 insertions(+), 3 deletions(-)
diff --git a/tests/plugin/inline.c b/tests/plugin/inline.c
index 30acc7a1838..8720ecc358a 100644
--- a/tests/plugin/inline.c
+++ b/tests/plugin/inline.c
@@ -20,8 +20,14 @@ typedef struct {
uint64_t count_insn_inline;
uint64_t count_mem;
uint64_t count_mem_inline;
+ uint64_t tb_cond_num_trigger;
+ uint64_t tb_cond_track_count;
+ uint64_t insn_cond_num_trigger;
+ uint64_t insn_cond_track_count;
} CPUCount;
+static const uint64_t cond_trigger_limit = 100;
+
typedef struct {
uint64_t data_insn;
uint64_t data_tb;
@@ -35,6 +41,10 @@ static qemu_plugin_u64 count_insn;
static qemu_plugin_u64 count_insn_inline;
static qemu_plugin_u64 count_mem;
static qemu_plugin_u64 count_mem_inline;
+static qemu_plugin_u64 tb_cond_num_trigger;
+static qemu_plugin_u64 tb_cond_track_count;
+static qemu_plugin_u64 insn_cond_num_trigger;
+static qemu_plugin_u64 insn_cond_track_count;
static struct qemu_plugin_scoreboard *data;
static qemu_plugin_u64 data_insn;
static qemu_plugin_u64 data_tb;
@@ -56,12 +66,19 @@ static void stats_insn(void)
const uint64_t per_vcpu = qemu_plugin_u64_sum(count_insn);
const uint64_t inl_per_vcpu =
qemu_plugin_u64_sum(count_insn_inline);
+ const uint64_t cond_num_trigger =
+ qemu_plugin_u64_sum(insn_cond_num_trigger);
+ const uint64_t cond_track_left = qemu_plugin_u64_sum(insn_cond_track_count);
+ const uint64_t conditional =
+ cond_num_trigger * cond_trigger_limit + cond_track_left;
printf("insn: %" PRIu64 "\n", expected);
printf("insn: %" PRIu64 " (per vcpu)\n", per_vcpu);
printf("insn: %" PRIu64 " (per vcpu inline)\n", inl_per_vcpu);
+ printf("insn: %" PRIu64 " (cond cb)\n", conditional);
g_assert(expected > 0);
g_assert(per_vcpu == expected);
g_assert(inl_per_vcpu == expected);
+ g_assert(conditional == expected);
}
static void stats_tb(void)
@@ -70,12 +87,18 @@ static void stats_tb(void)
const uint64_t per_vcpu = qemu_plugin_u64_sum(count_tb);
const uint64_t inl_per_vcpu =
qemu_plugin_u64_sum(count_tb_inline);
+ const uint64_t cond_num_trigger = qemu_plugin_u64_sum(tb_cond_num_trigger);
+ const uint64_t cond_track_left = qemu_plugin_u64_sum(tb_cond_track_count);
+ const uint64_t conditional =
+ cond_num_trigger * cond_trigger_limit + cond_track_left;
printf("tb: %" PRIu64 "\n", expected);
printf("tb: %" PRIu64 " (per vcpu)\n", per_vcpu);
printf("tb: %" PRIu64 " (per vcpu inline)\n", inl_per_vcpu);
+ printf("tb: %" PRIu64 " (conditional cb)\n", conditional);
g_assert(expected > 0);
g_assert(per_vcpu == expected);
g_assert(inl_per_vcpu == expected);
+ g_assert(conditional == expected);
}
static void stats_mem(void)
@@ -104,14 +127,35 @@ static void plugin_exit(qemu_plugin_id_t id, void *udata)
const uint64_t insn_inline = qemu_plugin_u64_get(count_insn_inline, i);
const uint64_t mem = qemu_plugin_u64_get(count_mem, i);
const uint64_t mem_inline = qemu_plugin_u64_get(count_mem_inline, i);
- printf("cpu %d: tb (%" PRIu64 ", %" PRIu64 ") | "
- "insn (%" PRIu64 ", %" PRIu64 ") | "
+ const uint64_t tb_cond_trigger =
+ qemu_plugin_u64_get(tb_cond_num_trigger, i);
+ const uint64_t tb_cond_left =
+ qemu_plugin_u64_get(tb_cond_track_count, i);
+ const uint64_t insn_cond_trigger =
+ qemu_plugin_u64_get(insn_cond_num_trigger, i);
+ const uint64_t insn_cond_left =
+ qemu_plugin_u64_get(insn_cond_track_count, i);
+ printf("cpu %d: tb (%" PRIu64 ", %" PRIu64
+ ", %" PRIu64 " * %" PRIu64 " + %" PRIu64
+ ") | "
+ "insn (%" PRIu64 ", %" PRIu64
+ ", %" PRIu64 " * %" PRIu64 " + %" PRIu64
+ ") | "
"mem (%" PRIu64 ", %" PRIu64 ")"
"\n",
- i, tb, tb_inline, insn, insn_inline, mem, mem_inline);
+ i,
+ tb, tb_inline,
+ tb_cond_trigger, cond_trigger_limit, tb_cond_left,
+ insn, insn_inline,
+ insn_cond_trigger, cond_trigger_limit, insn_cond_left,
+ mem, mem_inline);
g_assert(tb == tb_inline);
g_assert(insn == insn_inline);
g_assert(mem == mem_inline);
+ g_assert(tb_cond_trigger == tb / cond_trigger_limit);
+ g_assert(tb_cond_left == tb % cond_trigger_limit);
+ g_assert(insn_cond_trigger == insn / cond_trigger_limit);
+ g_assert(insn_cond_left == insn % cond_trigger_limit);
}
stats_tb();
@@ -132,6 +176,22 @@ static void vcpu_tb_exec(unsigned int cpu_index, void *udata)
g_mutex_unlock(&tb_lock);
}
+static void vcpu_tb_cond_exec(unsigned int cpu_index, void *udata)
+{
+ g_assert(qemu_plugin_u64_get(tb_cond_track_count, cpu_index) ==
+ cond_trigger_limit);
+ qemu_plugin_u64_set(tb_cond_track_count, cpu_index, 0);
+ qemu_plugin_u64_add(tb_cond_num_trigger, cpu_index, 1);
+}
+
+static void vcpu_insn_cond_exec(unsigned int cpu_index, void *udata)
+{
+ g_assert(qemu_plugin_u64_get(insn_cond_track_count, cpu_index) ==
+ cond_trigger_limit);
+ qemu_plugin_u64_set(insn_cond_track_count, cpu_index, 0);
+ qemu_plugin_u64_add(insn_cond_num_trigger, cpu_index, 1);
+}
+
static void vcpu_insn_exec(unsigned int cpu_index, void *udata)
{
qemu_plugin_u64_add(count_insn, cpu_index, 1);
@@ -163,6 +223,12 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
tb, QEMU_PLUGIN_INLINE_STORE_U64, data_tb, (uintptr_t) tb_store);
+ qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
+ tb, QEMU_PLUGIN_INLINE_ADD_U64, tb_cond_track_count, 1);
+ qemu_plugin_register_vcpu_tb_exec_cond_cb(
+ tb, vcpu_tb_cond_exec, QEMU_PLUGIN_CB_NO_REGS,
+ QEMU_PLUGIN_COND_EQ, tb_cond_track_count, cond_trigger_limit, NULL);
+
for (int idx = 0; idx < qemu_plugin_tb_n_insns(tb); ++idx) {
struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, idx);
void *insn_store = insn;
@@ -176,6 +242,13 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
insn, QEMU_PLUGIN_INLINE_ADD_U64, count_insn_inline, 1);
+ qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
+ insn, QEMU_PLUGIN_INLINE_ADD_U64, insn_cond_track_count, 1);
+ qemu_plugin_register_vcpu_insn_exec_cond_cb(
+ insn, vcpu_insn_cond_exec, QEMU_PLUGIN_CB_NO_REGS,
+ QEMU_PLUGIN_COND_EQ, insn_cond_track_count, cond_trigger_limit,
+ NULL);
+
qemu_plugin_register_vcpu_mem_cb(insn, &vcpu_mem_access,
QEMU_PLUGIN_CB_NO_REGS,
QEMU_PLUGIN_MEM_RW, mem_store);
@@ -207,6 +280,14 @@ int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info,
counts, CPUCount, count_insn_inline);
count_mem_inline = qemu_plugin_scoreboard_u64_in_struct(
counts, CPUCount, count_mem_inline);
+ tb_cond_num_trigger = qemu_plugin_scoreboard_u64_in_struct(
+ counts, CPUCount, tb_cond_num_trigger);
+ tb_cond_track_count = qemu_plugin_scoreboard_u64_in_struct(
+ counts, CPUCount, tb_cond_track_count);
+ insn_cond_num_trigger = qemu_plugin_scoreboard_u64_in_struct(
+ counts, CPUCount, insn_cond_num_trigger);
+ insn_cond_track_count = qemu_plugin_scoreboard_u64_in_struct(
+ counts, CPUCount, insn_cond_track_count);
data = qemu_plugin_scoreboard_new(sizeof(CPUData));
data_insn = qemu_plugin_scoreboard_u64_in_struct(data, CPUData, data_insn);
data_tb = qemu_plugin_scoreboard_u64_in_struct(data, CPUData, data_tb);
--
2.43.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 0/5] TCG plugins new inline operations
2024-02-29 5:53 [PATCH 0/5] TCG plugins new inline operations Pierrick Bouvier
` (4 preceding siblings ...)
2024-02-29 5:53 ` [PATCH 5/5] tests/plugin/inline: add test for condition callback Pierrick Bouvier
@ 2024-03-08 10:41 ` Pierrick Bouvier
5 siblings, 0 replies; 14+ messages in thread
From: Pierrick Bouvier @ 2024-03-08 10:41 UTC (permalink / raw)
To: qemu-devel
Cc: Mahmoud Mandour, Paolo Bonzini, Alexandre Iooss, Alex Bennée,
Richard Henderson
Hi everyone,
polite ping for this series, that might have a chance to be included for
9.0 soft-freeze.
Regards,
Pierrick
On 2/29/24 09:53, Pierrick Bouvier wrote:
> This series implement two new operations for plugins:
> - Store inline allows to write a specific value to a scoreboard.
> - Conditional callback executes a callback only when a given condition is true.
> The condition is evaluated inline.
>
> It's possible to mix various inline operations (add, store) with conditional
> callbacks, allowing efficient "trap" based counters.
>
> It builds on top of new scoreboard API, introduced in the previous series.
>
> Based-on: 20240229052506.933222-1-pierrick.bouvier@linaro.org
>
> Pierrick Bouvier (5):
> plugins: prepare introduction of new inline ops
> plugins: add new inline op STORE_U64
> tests/plugin/inline: add test for STORE_U64 inline op
> plugins: conditional callbacks
> tests/plugin/inline: add test for condition callback
>
> include/qemu/plugin.h | 10 +-
> include/qemu/qemu-plugin.h | 80 +++++++-
> plugins/plugin.h | 9 +
> accel/tcg/plugin-gen.c | 359 +++++++++++++++++++++++++++++++----
> plugins/api.c | 76 +++++++-
> plugins/core.c | 28 ++-
> tests/plugin/inline.c | 128 ++++++++++++-
> plugins/qemu-plugins.symbols | 2 +
> 8 files changed, 633 insertions(+), 59 deletions(-)
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5] plugins: conditional callbacks
2024-02-29 5:53 ` [PATCH 4/5] plugins: conditional callbacks Pierrick Bouvier
@ 2024-03-11 10:08 ` Alex Bennée
2024-03-12 6:03 ` Pierrick Bouvier
2024-03-11 15:43 ` Alex Bennée
1 sibling, 1 reply; 14+ messages in thread
From: Alex Bennée @ 2024-03-11 10:08 UTC (permalink / raw)
To: Pierrick Bouvier
Cc: qemu-devel, Mahmoud Mandour, Paolo Bonzini, Alexandre Iooss,
Richard Henderson
Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
> Extend plugins API to support callback called with a given criteria
> (evaluated inline).
>
> Added functions:
> - qemu_plugin_register_vcpu_tb_exec_cond_cb
> - qemu_plugin_register_vcpu_insn_exec_cond_cb
>
> They expect as parameter a condition, a qemu_plugin_u64_t (op1) and an
> immediate (op2). Callback is called if op1 |cond| op2 is true.
>
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
> include/qemu/plugin.h | 7 ++
> include/qemu/qemu-plugin.h | 76 +++++++++++++++
> plugins/plugin.h | 8 ++
> accel/tcg/plugin-gen.c | 174 ++++++++++++++++++++++++++++++++++-
> plugins/api.c | 51 ++++++++++
> plugins/core.c | 19 ++++
> plugins/qemu-plugins.symbols | 2 +
> 7 files changed, 334 insertions(+), 3 deletions(-)
>
> diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
> index d92d64744e6..056102b2361 100644
> --- a/include/qemu/plugin.h
> +++ b/include/qemu/plugin.h
> @@ -74,6 +74,8 @@ enum plugin_dyn_cb_type {
> enum plugin_dyn_cb_subtype {
> PLUGIN_CB_REGULAR,
> PLUGIN_CB_REGULAR_R,
> + PLUGIN_CB_COND,
> + PLUGIN_CB_COND_R,
> PLUGIN_CB_INLINE_ADD_U64,
> PLUGIN_CB_INLINE_STORE_U64,
> PLUGIN_N_CB_SUBTYPES,
> @@ -97,6 +99,11 @@ struct qemu_plugin_dyn_cb {
> enum qemu_plugin_op op;
> uint64_t imm;
> } inline_insn;
> + struct {
> + qemu_plugin_u64 entry;
> + enum qemu_plugin_cond cond;
> + uint64_t imm;
> + } cond_cb;
> };
> };
>
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index c5cac897a0b..337de25ece7 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -262,6 +262,29 @@ enum qemu_plugin_mem_rw {
> QEMU_PLUGIN_MEM_RW,
> };
>
> +/**
> + * enum qemu_plugin_cond - condition to enable callback
> + *
> + * @QEMU_PLUGIN_COND_NEVER: false
> + * @QEMU_PLUGIN_COND_ALWAYS: true
> + * @QEMU_PLUGIN_COND_EQ: is equal?
> + * @QEMU_PLUGIN_COND_NE: is not equal?
> + * @QEMU_PLUGIN_COND_LT: is less than?
> + * @QEMU_PLUGIN_COND_LE: is less than or equal?
> + * @QEMU_PLUGIN_COND_GT: is greater than?
> + * @QEMU_PLUGIN_COND_GE: is greater than or equal?
> + */
> +enum qemu_plugin_cond {
> + QEMU_PLUGIN_COND_NEVER,
> + QEMU_PLUGIN_COND_ALWAYS,
> + QEMU_PLUGIN_COND_EQ,
> + QEMU_PLUGIN_COND_NE,
> + QEMU_PLUGIN_COND_LT,
> + QEMU_PLUGIN_COND_LE,
> + QEMU_PLUGIN_COND_GT,
> + QEMU_PLUGIN_COND_GE,
> +};
> +
> /**
> * typedef qemu_plugin_vcpu_tb_trans_cb_t - translation callback
> * @id: unique plugin id
> @@ -301,6 +324,32 @@ void qemu_plugin_register_vcpu_tb_exec_cb(struct qemu_plugin_tb *tb,
> enum qemu_plugin_cb_flags flags,
> void *userdata);
>
> +/**
> + * qemu_plugin_register_vcpu_tb_exec_cond_cb() - register conditional callback
> + * @tb: the opaque qemu_plugin_tb handle for the translation
> + * @cb: callback function
> + * @cond: condition to enable callback
> + * @entry: first operand for condition
> + * @imm: second operand for condition
> + * @flags: does the plugin read or write the CPU's registers?
> + * @userdata: any plugin data to pass to the @cb?
> + *
> + * The @cb function is called when a translated unit executes if
> + * entry @cond imm is true.
> + * If condition is QEMU_PLUGIN_COND_ALWAYS, condition is never interpreted and
> + * this function is equivalent to qemu_plugin_register_vcpu_tb_exec_cb.
> + * If condition QEMU_PLUGIN_COND_NEVER, condition is never interpreted and
> + * callback is never installed.
> + */
> +QEMU_PLUGIN_API
> +void qemu_plugin_register_vcpu_tb_exec_cond_cb(struct qemu_plugin_tb *tb,
> + qemu_plugin_vcpu_udata_cb_t cb,
> + enum qemu_plugin_cb_flags flags,
> + enum qemu_plugin_cond cond,
> + qemu_plugin_u64 entry,
Is this a fixed entry or part of a scoreboard?
I'm trying to write a control flow plugin with a structure like:
/* We use this to track the current execution state */
typedef struct {
/* address of start of block */
uint64_t block_start;
/* address of end of block */
uint64_t block_end;
/* address of last executed PC */
uint64_t last_pc;
} VCPUScoreBoard;
And I want to check to see if last_pc (set by STORE_U64 for each insn) == block_end
(set at start of TB with what we know).
Is this something I need to get with:
last_pc = qemu_plugin_scoreboard_u64_in_struct(state, VCPUScoreBoard, last_pc);
?
> + uint64_t imm,
> + void *userdata);
> +
> /**
> * enum qemu_plugin_op - describes an inline op
> *
> @@ -344,6 +393,33 @@ void qemu_plugin_register_vcpu_insn_exec_cb(struct qemu_plugin_insn *insn,
> enum qemu_plugin_cb_flags flags,
> void *userdata);
>
> +/**
> + * qemu_plugin_register_vcpu_insn_exec_cond_cb() - conditional insn execution cb
> + * @insn: the opaque qemu_plugin_insn handle for an instruction
> + * @cb: callback function
> + * @flags: does the plugin read or write the CPU's registers?
> + * @cond: condition to enable callback
> + * @entry: first operand for condition
> + * @imm: second operand for condition
> + * @userdata: any plugin data to pass to the @cb?
> + *
> + * The @cb function is called when an instruction executes if
> + * entry @cond imm is true.
> + * If condition is QEMU_PLUGIN_COND_ALWAYS, condition is never interpreted and
> + * this function is equivalent to qemu_plugin_register_vcpu_insn_exec_cb.
> + * If condition QEMU_PLUGIN_COND_NEVER, condition is never interpreted and
> + * callback is never installed.
> + */
> +QEMU_PLUGIN_API
> +void qemu_plugin_register_vcpu_insn_exec_cond_cb(
> + struct qemu_plugin_insn *insn,
> + qemu_plugin_vcpu_udata_cb_t cb,
> + enum qemu_plugin_cb_flags flags,
> + enum qemu_plugin_cond cond,
> + qemu_plugin_u64 entry,
> + uint64_t imm,
> + void *userdata);
> +
> /**
> * qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu() - insn exec inline op
> * @insn: the opaque qemu_plugin_insn handle for an instruction
> diff --git a/plugins/plugin.h b/plugins/plugin.h
> index 696b1fa38b0..118cc99bb2a 100644
> --- a/plugins/plugin.h
> +++ b/plugins/plugin.h
> @@ -94,6 +94,14 @@ plugin_register_dyn_cb__udata(GArray **arr,
> qemu_plugin_vcpu_udata_cb_t cb,
> enum qemu_plugin_cb_flags flags, void *udata);
>
> +void
> +plugin_register_dyn_cond_cb__udata(GArray **arr,
> + qemu_plugin_vcpu_udata_cb_t cb,
> + enum qemu_plugin_cb_flags flags,
> + enum qemu_plugin_cond cond,
> + qemu_plugin_u64 entry,
> + uint64_t imm,
> + void *udata);
>
> void plugin_register_vcpu_mem_cb(GArray **arr,
> void *cb,
> diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
> index 02c894106e2..15762a26439 100644
> --- a/accel/tcg/plugin-gen.c
> +++ b/accel/tcg/plugin-gen.c
> @@ -46,6 +46,7 @@
> #include "qemu/plugin.h"
> #include "cpu.h"
> #include "tcg/tcg.h"
> +#include "tcg/tcg-cond.h"
> #include "tcg/tcg-internal.h"
> #include "tcg/tcg-op.h"
> #include "tcg/tcg-temp-internal.h"
> @@ -82,6 +83,8 @@ enum plugin_gen_from {
> enum plugin_gen_cb {
> PLUGIN_GEN_CB_UDATA,
> PLUGIN_GEN_CB_UDATA_R,
> + PLUGIN_GEN_CB_COND_UDATA,
> + PLUGIN_GEN_CB_COND_UDATA_R,
> PLUGIN_GEN_CB_INLINE_ADD_U64,
> PLUGIN_GEN_CB_INLINE_STORE_U64,
> PLUGIN_GEN_CB_MEM,
> @@ -119,16 +122,58 @@ static void gen_empty_udata_cb(void (*gen_helper)(TCGv_i32, TCGv_ptr))
> tcg_temp_free_i32(cpu_index);
> }
>
> +static void gen_empty_cond_udata_cb(void (*gen_helper)(TCGv_i32, TCGv_ptr))
> +{
> + TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
> + TCGv_i32 cpu_offset = tcg_temp_ebb_new_i32();
> + TCGv_ptr cpu_index_as_ptr = tcg_temp_ebb_new_ptr();
> + TCGv_i64 val = tcg_temp_ebb_new_i64();
> + TCGv_ptr ptr = tcg_temp_ebb_new_ptr();
> + TCGv_ptr udata = tcg_temp_ebb_new_ptr();
> + TCGLabel *after_cb = gen_new_label();
> +
> + tcg_gen_movi_ptr(udata, 0);
> + tcg_gen_ld_i32(cpu_index, tcg_env,
> + -offsetof(ArchCPU, env) + offsetof(CPUState, cpu_index));
> + /* second operand will be replaced by immediate value */
> + tcg_gen_mul_i32(cpu_offset, cpu_index, cpu_index);
> + tcg_gen_ext_i32_ptr(cpu_index_as_ptr, cpu_offset);
> + tcg_gen_movi_ptr(ptr, 0);
> + tcg_gen_add_ptr(ptr, ptr, cpu_index_as_ptr);
> + tcg_gen_ld_i64(val, ptr, 0);
> +
> + tcg_gen_brcondi_i64(TCG_COND_EQ, val, 0, after_cb);
> + gen_helper(cpu_index, udata);
> + gen_set_label(after_cb);
> +
> + tcg_temp_free_ptr(udata);
> + tcg_temp_free_ptr(ptr);
> + tcg_temp_free_i64(val);
> + tcg_temp_free_ptr(cpu_index_as_ptr);
> + tcg_temp_free_i32(cpu_offset);
> + tcg_temp_free_i32(cpu_index);
> +}
> +
> static void gen_empty_udata_cb_no_wg(void)
> {
> gen_empty_udata_cb(gen_helper_plugin_vcpu_udata_cb_no_wg);
> }
>
> +static void gen_empty_cond_udata_cb_no_wg(void)
> +{
> + gen_empty_cond_udata_cb(gen_helper_plugin_vcpu_udata_cb_no_wg);
> +}
> +
> static void gen_empty_udata_cb_no_rwg(void)
> {
> gen_empty_udata_cb(gen_helper_plugin_vcpu_udata_cb_no_rwg);
> }
>
> +static void gen_empty_cond_udata_cb_no_rwg(void)
> +{
> + gen_empty_cond_udata_cb(gen_helper_plugin_vcpu_udata_cb_no_rwg);
> +}
> +
> static void gen_empty_inline_cb_add_u64(void)
> {
> TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
> @@ -248,6 +293,10 @@ static void plugin_gen_empty_callback(enum plugin_gen_from from)
> gen_empty_inline_cb_store_u64);
> gen_wrapped(from, PLUGIN_GEN_CB_UDATA, gen_empty_udata_cb_no_rwg);
> gen_wrapped(from, PLUGIN_GEN_CB_UDATA_R, gen_empty_udata_cb_no_wg);
> + gen_wrapped(from, PLUGIN_GEN_CB_COND_UDATA,
> + gen_empty_cond_udata_cb_no_rwg);
> + gen_wrapped(from, PLUGIN_GEN_CB_COND_UDATA_R,
> + gen_empty_cond_udata_cb_no_wg);
> break;
> default:
> g_assert_not_reached();
> @@ -456,6 +505,29 @@ static TCGOp *copy_call(TCGOp **begin_op, TCGOp *op, void *func, int *cb_idx)
> return op;
> }
>
> +static TCGOp *copy_brcondi_i64(TCGOp **begin_op, TCGOp *op, TCGCond cond,
> + uint64_t imm)
> +{
> + if (TCG_TARGET_REG_BITS == 32) {
> + op = copy_op(begin_op, op, INDEX_op_brcond2_i32);
> + /* low(arg1), high(arg1), 32(arg2), 32(arg2 >> 32), cond, label */
> + op->args[2] = tcgv_i32_arg(tcg_constant_i32(imm));
> + op->args[3] = tcgv_i32_arg(tcg_constant_i32(imm >> 32));
> + op->args[4] = cond;
> + } else {
> + op = copy_op(begin_op, op, INDEX_op_brcond_i64);
> + /* arg1, arg2, cond, label */
> + op->args[1] = tcgv_i64_arg(tcg_constant_i64(imm));
> + op->args[2] = cond;
> + }
> + return op;
> +}
> +
> +static TCGOp *copy_set_label(TCGOp **begin_op, TCGOp *op)
> +{
> + return copy_op(begin_op, op, INDEX_op_set_label);
> +}
> +
> /*
> * When we append/replace ops here we are sensitive to changing patterns of
> * TCGOps generated by the tcg_gen_FOO calls when we generated the
> @@ -482,6 +554,50 @@ static TCGOp *append_udata_cb(const struct qemu_plugin_dyn_cb *cb,
> return op;
> }
>
> +static TCGCond plugin_cond_to_tcgcond(enum qemu_plugin_cond cond)
> +{
> + switch (cond) {
> + case QEMU_PLUGIN_COND_EQ:
> + return TCG_COND_EQ;
> + case QEMU_PLUGIN_COND_NE:
> + return TCG_COND_NE;
> + case QEMU_PLUGIN_COND_LT:
> + return TCG_COND_LTU;
> + case QEMU_PLUGIN_COND_LE:
> + return TCG_COND_LEU;
> + case QEMU_PLUGIN_COND_GT:
> + return TCG_COND_GTU;
> + case QEMU_PLUGIN_COND_GE:
> + return TCG_COND_GEU;
> + default:
> + /* ALWAYS and NEVER conditions should never reach */
> + g_assert_not_reached();
> + }
> +}
> +
> +static TCGOp *append_cond_udata_cb(const struct qemu_plugin_dyn_cb *cb,
> + TCGOp *begin_op, TCGOp *op, int *cb_idx)
> +{
> + char *ptr = cb->cond_cb.entry.score->data->data;
> + size_t elem_size = g_array_get_element_size(
> + cb->cond_cb.entry.score->data);
> + size_t offset = cb->cond_cb.entry.offset;
> + /* Condition should be negated, as calling the cb is the "else" path */
> + TCGCond cond = tcg_invert_cond(plugin_cond_to_tcgcond(cb->cond_cb.cond));
> +
> + op = copy_const_ptr(&begin_op, op, ptr);
> + op = copy_ld_i32(&begin_op, op);
> + op = copy_mul_i32(&begin_op, op, elem_size);
> + op = copy_ext_i32_ptr(&begin_op, op);
> + op = copy_const_ptr(&begin_op, op, ptr + offset);
> + op = copy_add_ptr(&begin_op, op);
> + op = copy_ld_i64(&begin_op, op);
> + op = copy_brcondi_i64(&begin_op, op, cond, cb->cond_cb.imm);
> + op = copy_call(&begin_op, op, cb->f.vcpu_udata, cb_idx);
> + op = copy_set_label(&begin_op, op);
> + return op;
> +}
> +
> static TCGOp *append_inline_cb_add_u64(const struct qemu_plugin_dyn_cb *cb,
> TCGOp *begin_op, TCGOp *op,
> int *unused)
> @@ -601,6 +717,12 @@ inject_udata_cb(const GArray *cbs, TCGOp *begin_op)
> inject_cb_type(cbs, begin_op, append_udata_cb, op_ok);
> }
>
> +static void
> +inject_cond_udata_cb(const GArray *cbs, TCGOp *begin_op)
> +{
> + inject_cb_type(cbs, begin_op, append_cond_udata_cb, op_ok);
> +}
> +
> static void
> inject_inline_cb_add_u64(const GArray *cbs, TCGOp *begin_op, op_ok_fn ok)
> {
> @@ -654,7 +776,7 @@ static void inject_mem_enable_helper(struct qemu_plugin_tb *ptb,
> struct qemu_plugin_insn *plugin_insn,
> TCGOp *begin_op)
> {
> - GArray *cbs[3];
> + GArray *cbs[4];
> GArray *arr;
> size_t n_cbs, i;
>
> @@ -662,6 +784,7 @@ static void inject_mem_enable_helper(struct qemu_plugin_tb *ptb,
> cbs[0] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_INLINE_ADD_U64];
> cbs[1] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_INLINE_STORE_U64];
> cbs[2] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_REGULAR];
> + cbs[3] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_COND];
>
> n_cbs = 0;
> for (i = 0; i < ARRAY_SIZE(cbs); i++) {
> @@ -727,6 +850,18 @@ static void plugin_gen_tb_udata_r(const struct qemu_plugin_tb *ptb,
> inject_udata_cb(ptb->cbs[PLUGIN_CB_REGULAR_R], begin_op);
> }
>
> +static void plugin_gen_tb_cond_udata(const struct qemu_plugin_tb *ptb,
> + TCGOp *begin_op)
> +{
> + inject_cond_udata_cb(ptb->cbs[PLUGIN_CB_COND], begin_op);
> +}
> +
> +static void plugin_gen_tb_cond_udata_r(const struct qemu_plugin_tb *ptb,
> + TCGOp *begin_op)
> +{
> + inject_cond_udata_cb(ptb->cbs[PLUGIN_CB_COND_R], begin_op);
> +}
> +
> static void plugin_gen_tb_inline_add_u64(const struct qemu_plugin_tb *ptb,
> TCGOp *begin_op)
> {
> @@ -745,7 +880,6 @@ static void plugin_gen_insn_udata(const struct qemu_plugin_tb *ptb,
> TCGOp *begin_op, int insn_idx)
> {
> struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
> -
> inject_udata_cb(insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_REGULAR], begin_op);
> }
>
> @@ -753,10 +887,23 @@ static void plugin_gen_insn_udata_r(const struct qemu_plugin_tb *ptb,
> TCGOp *begin_op, int insn_idx)
> {
> struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
> -
> inject_udata_cb(insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_REGULAR_R], begin_op);
> }
>
> +static void plugin_gen_insn_cond_udata(const struct qemu_plugin_tb *ptb,
> + TCGOp *begin_op, int insn_idx)
> +{
> + struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
> + inject_cond_udata_cb(insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_COND], begin_op);
> +}
> +
> +static void plugin_gen_insn_cond_udata_r(const struct qemu_plugin_tb *ptb,
> + TCGOp *begin_op, int insn_idx)
> +{
> + struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
> + inject_cond_udata_cb(insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_COND_R], begin_op);
> +}
> +
> static void plugin_gen_insn_inline_add_u64(const struct qemu_plugin_tb *ptb,
> TCGOp *begin_op, int insn_idx)
> {
> @@ -843,6 +990,15 @@ static void pr_ops(void)
> case PLUGIN_GEN_CB_UDATA:
> type = "udata";
> break;
> + case PLUGIN_GEN_CB_UDATA_R:
> + type = "udata (read)";
> + break;
> + case PLUGIN_GEN_CB_COND_UDATA:
> + type = "cond udata";
> + break;
> + case PLUGIN_GEN_CB_COND_UDATA_R:
> + type = "cond udata (read)";
> + break;
> case PLUGIN_GEN_CB_INLINE_ADD_U64:
> type = "inline add u64";
> break;
> @@ -897,6 +1053,12 @@ static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
> case PLUGIN_GEN_CB_UDATA_R:
> plugin_gen_tb_udata_r(plugin_tb, op);
> break;
> + case PLUGIN_GEN_CB_COND_UDATA:
> + plugin_gen_tb_cond_udata(plugin_tb, op);
> + break;
> + case PLUGIN_GEN_CB_COND_UDATA_R:
> + plugin_gen_tb_cond_udata_r(plugin_tb, op);
> + break;
> case PLUGIN_GEN_CB_INLINE_ADD_U64:
> plugin_gen_tb_inline_add_u64(plugin_tb, op);
> break;
> @@ -919,6 +1081,12 @@ static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
> case PLUGIN_GEN_CB_UDATA_R:
> plugin_gen_insn_udata_r(plugin_tb, op, insn_idx);
> break;
> + case PLUGIN_GEN_CB_COND_UDATA:
> + plugin_gen_insn_cond_udata(plugin_tb, op, insn_idx);
> + break;
> + case PLUGIN_GEN_CB_COND_UDATA_R:
> + plugin_gen_insn_cond_udata_r(plugin_tb, op, insn_idx);
> + break;
> case PLUGIN_GEN_CB_INLINE_ADD_U64:
> plugin_gen_insn_inline_add_u64(plugin_tb, op, insn_idx);
> break;
> diff --git a/plugins/api.c b/plugins/api.c
> index 4a033c16f83..f24d68d7a68 100644
> --- a/plugins/api.c
> +++ b/plugins/api.c
> @@ -113,6 +113,31 @@ void qemu_plugin_register_vcpu_tb_exec_cb(struct qemu_plugin_tb *tb,
> }
> }
>
> +void qemu_plugin_register_vcpu_tb_exec_cond_cb(struct qemu_plugin_tb *tb,
> + qemu_plugin_vcpu_udata_cb_t cb,
> + enum qemu_plugin_cb_flags flags,
> + enum qemu_plugin_cond cond,
> + qemu_plugin_u64 entry,
> + uint64_t imm,
> + void *udata)
> +{
> + if (cond == QEMU_PLUGIN_COND_NEVER || tb->mem_only) {
> + return;
> + }
> + if (cond == QEMU_PLUGIN_COND_ALWAYS) {
> + qemu_plugin_register_vcpu_tb_exec_cb(tb, cb, flags, udata);
> + return;
> + }
> + int index = flags == QEMU_PLUGIN_CB_R_REGS ||
> + flags == QEMU_PLUGIN_CB_RW_REGS ?
> + PLUGIN_CB_COND_R : PLUGIN_CB_COND;
> +
> + plugin_register_dyn_cond_cb__udata(&tb->cbs[index],
> + cb, flags,
> + cond, entry, imm,
> + udata);
> +}
> +
> void qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
> struct qemu_plugin_tb *tb,
> enum qemu_plugin_op op,
> @@ -141,6 +166,32 @@ void qemu_plugin_register_vcpu_insn_exec_cb(struct qemu_plugin_insn *insn,
> }
> }
>
> +void qemu_plugin_register_vcpu_insn_exec_cond_cb(
> + struct qemu_plugin_insn *insn,
> + qemu_plugin_vcpu_udata_cb_t cb,
> + enum qemu_plugin_cb_flags flags,
> + enum qemu_plugin_cond cond,
> + qemu_plugin_u64 entry,
> + uint64_t imm,
> + void *udata)
> +{
> + if (cond == QEMU_PLUGIN_COND_NEVER || insn->mem_only) {
> + return;
> + }
> + if (cond == QEMU_PLUGIN_COND_ALWAYS) {
> + qemu_plugin_register_vcpu_insn_exec_cb(insn, cb, flags, udata);
> + return;
> + }
> + int index = flags == QEMU_PLUGIN_CB_R_REGS ||
> + flags == QEMU_PLUGIN_CB_RW_REGS ?
> + PLUGIN_CB_COND_R : PLUGIN_CB_COND;
> +
> + plugin_register_dyn_cond_cb__udata(&insn->cbs[PLUGIN_CB_INSN][index],
> + cb, flags,
> + cond, entry, imm,
> + udata);
> +}
> +
> void qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
> struct qemu_plugin_insn *insn,
> enum qemu_plugin_op op,
> diff --git a/plugins/core.c b/plugins/core.c
> index 11f72594229..82d5f17438e 100644
> --- a/plugins/core.c
> +++ b/plugins/core.c
> @@ -347,6 +347,25 @@ void plugin_register_dyn_cb__udata(GArray **arr,
> dyn_cb->type = PLUGIN_CB_REGULAR;
> }
>
> +void plugin_register_dyn_cond_cb__udata(GArray **arr,
> + qemu_plugin_vcpu_udata_cb_t cb,
> + enum qemu_plugin_cb_flags flags,
> + enum qemu_plugin_cond cond,
> + qemu_plugin_u64 entry,
> + uint64_t imm,
> + void *udata)
> +{
> + struct qemu_plugin_dyn_cb *dyn_cb = plugin_get_dyn_cb(arr);
> +
> + dyn_cb->userp = udata;
> + /* Note flags are discarded as unused. */
> + dyn_cb->f.vcpu_udata = cb;
> + dyn_cb->type = PLUGIN_CB_COND;
> + dyn_cb->cond_cb.cond = cond;
> + dyn_cb->cond_cb.entry = entry;
> + dyn_cb->cond_cb.imm = imm;
> +}
> +
> void plugin_register_vcpu_mem_cb(GArray **arr,
> void *cb,
> enum qemu_plugin_cb_flags flags,
> diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols
> index a9fac056c7f..aa0a77a319f 100644
> --- a/plugins/qemu-plugins.symbols
> +++ b/plugins/qemu-plugins.symbols
> @@ -27,6 +27,7 @@
> qemu_plugin_register_vcpu_idle_cb;
> qemu_plugin_register_vcpu_init_cb;
> qemu_plugin_register_vcpu_insn_exec_cb;
> + qemu_plugin_register_vcpu_insn_exec_cond_cb;
> qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu;
> qemu_plugin_register_vcpu_mem_cb;
> qemu_plugin_register_vcpu_mem_inline_per_vcpu;
> @@ -34,6 +35,7 @@
> qemu_plugin_register_vcpu_syscall_cb;
> qemu_plugin_register_vcpu_syscall_ret_cb;
> qemu_plugin_register_vcpu_tb_exec_cb;
> + qemu_plugin_register_vcpu_tb_exec_cond_cb;
> qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu;
> qemu_plugin_register_vcpu_tb_trans_cb;
> qemu_plugin_reset;
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5] plugins: conditional callbacks
2024-02-29 5:53 ` [PATCH 4/5] plugins: conditional callbacks Pierrick Bouvier
2024-03-11 10:08 ` Alex Bennée
@ 2024-03-11 15:43 ` Alex Bennée
2024-03-12 6:03 ` Pierrick Bouvier
1 sibling, 1 reply; 14+ messages in thread
From: Alex Bennée @ 2024-03-11 15:43 UTC (permalink / raw)
To: Pierrick Bouvier
Cc: qemu-devel, Mahmoud Mandour, Paolo Bonzini, Alexandre Iooss,
Richard Henderson
Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
> Extend plugins API to support callback called with a given criteria
> (evaluated inline).
>
> Added functions:
> - qemu_plugin_register_vcpu_tb_exec_cond_cb
> - qemu_plugin_register_vcpu_insn_exec_cond_cb
>
> They expect as parameter a condition, a qemu_plugin_u64_t (op1) and an
> immediate (op2). Callback is called if op1 |cond| op2 is true.
>
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
<snip>
>
> +static TCGCond plugin_cond_to_tcgcond(enum qemu_plugin_cond cond)
> +{
> + switch (cond) {
> + case QEMU_PLUGIN_COND_EQ:
> + return TCG_COND_EQ;
> + case QEMU_PLUGIN_COND_NE:
> + return TCG_COND_NE;
> + case QEMU_PLUGIN_COND_LT:
> + return TCG_COND_LTU;
> + case QEMU_PLUGIN_COND_LE:
> + return TCG_COND_LEU;
> + case QEMU_PLUGIN_COND_GT:
> + return TCG_COND_GTU;
> + case QEMU_PLUGIN_COND_GE:
> + return TCG_COND_GEU;
> + default:
> + /* ALWAYS and NEVER conditions should never reach */
> + g_assert_not_reached();
> + }
> +}
> +
> +static TCGOp *append_cond_udata_cb(const struct qemu_plugin_dyn_cb *cb,
> + TCGOp *begin_op, TCGOp *op, int *cb_idx)
> +{
> + char *ptr = cb->cond_cb.entry.score->data->data;
> + size_t elem_size = g_array_get_element_size(
> + cb->cond_cb.entry.score->data);
> + size_t offset = cb->cond_cb.entry.offset;
> + /* Condition should be negated, as calling the cb is the "else" path */
> + TCGCond cond = tcg_invert_cond(plugin_cond_to_tcgcond(cb->cond_cb.cond));
> +
> + op = copy_const_ptr(&begin_op, op, ptr);
> + op = copy_ld_i32(&begin_op, op);
> + op = copy_mul_i32(&begin_op, op, elem_size);
> + op = copy_ext_i32_ptr(&begin_op, op);
> + op = copy_const_ptr(&begin_op, op, ptr + offset);
> + op = copy_add_ptr(&begin_op, op);
> + op = copy_ld_i64(&begin_op, op);
> + op = copy_brcondi_i64(&begin_op, op, cond, cb->cond_cb.imm);
> + op = copy_call(&begin_op, op, cb->f.vcpu_udata, cb_idx);
> + op = copy_set_label(&begin_op, op);
> + return op;
I think we are missing something here to ensure that udata is set
correctly for the callback, see my RFC:
Subject: [RFC PATCH] contrib/plugins: control flow plugin (WIP!)
Date: Mon, 11 Mar 2024 15:34:32 +0000
Message-Id: <20240311153432.1395190-1-alex.bennee@linaro.org>
which is seeing the same value every time in the callback.
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5] plugins: conditional callbacks
2024-03-11 10:08 ` Alex Bennée
@ 2024-03-12 6:03 ` Pierrick Bouvier
2024-03-12 15:04 ` Alex Bennée
0 siblings, 1 reply; 14+ messages in thread
From: Pierrick Bouvier @ 2024-03-12 6:03 UTC (permalink / raw)
To: Alex Bennée
Cc: qemu-devel, Mahmoud Mandour, Paolo Bonzini, Alexandre Iooss,
Richard Henderson
On 3/11/24 14:08, Alex Bennée wrote:
> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
>
>> Extend plugins API to support callback called with a given criteria
>> (evaluated inline).
>>
>> Added functions:
>> - qemu_plugin_register_vcpu_tb_exec_cond_cb
>> - qemu_plugin_register_vcpu_insn_exec_cond_cb
>>
>> They expect as parameter a condition, a qemu_plugin_u64_t (op1) and an
>> immediate (op2). Callback is called if op1 |cond| op2 is true.
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>> include/qemu/plugin.h | 7 ++
>> include/qemu/qemu-plugin.h | 76 +++++++++++++++
>> plugins/plugin.h | 8 ++
>> accel/tcg/plugin-gen.c | 174 ++++++++++++++++++++++++++++++++++-
>> plugins/api.c | 51 ++++++++++
>> plugins/core.c | 19 ++++
>> plugins/qemu-plugins.symbols | 2 +
>> 7 files changed, 334 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
>> index d92d64744e6..056102b2361 100644
>> --- a/include/qemu/plugin.h
>> +++ b/include/qemu/plugin.h
>> @@ -74,6 +74,8 @@ enum plugin_dyn_cb_type {
>> enum plugin_dyn_cb_subtype {
>> PLUGIN_CB_REGULAR,
>> PLUGIN_CB_REGULAR_R,
>> + PLUGIN_CB_COND,
>> + PLUGIN_CB_COND_R,
>> PLUGIN_CB_INLINE_ADD_U64,
>> PLUGIN_CB_INLINE_STORE_U64,
>> PLUGIN_N_CB_SUBTYPES,
>> @@ -97,6 +99,11 @@ struct qemu_plugin_dyn_cb {
>> enum qemu_plugin_op op;
>> uint64_t imm;
>> } inline_insn;
>> + struct {
>> + qemu_plugin_u64 entry;
>> + enum qemu_plugin_cond cond;
>> + uint64_t imm;
>> + } cond_cb;
>> };
>> };
>>
>> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
>> index c5cac897a0b..337de25ece7 100644
>> --- a/include/qemu/qemu-plugin.h
>> +++ b/include/qemu/qemu-plugin.h
>> @@ -262,6 +262,29 @@ enum qemu_plugin_mem_rw {
>> QEMU_PLUGIN_MEM_RW,
>> };
>>
>> +/**
>> + * enum qemu_plugin_cond - condition to enable callback
>> + *
>> + * @QEMU_PLUGIN_COND_NEVER: false
>> + * @QEMU_PLUGIN_COND_ALWAYS: true
>> + * @QEMU_PLUGIN_COND_EQ: is equal?
>> + * @QEMU_PLUGIN_COND_NE: is not equal?
>> + * @QEMU_PLUGIN_COND_LT: is less than?
>> + * @QEMU_PLUGIN_COND_LE: is less than or equal?
>> + * @QEMU_PLUGIN_COND_GT: is greater than?
>> + * @QEMU_PLUGIN_COND_GE: is greater than or equal?
>> + */
>> +enum qemu_plugin_cond {
>> + QEMU_PLUGIN_COND_NEVER,
>> + QEMU_PLUGIN_COND_ALWAYS,
>> + QEMU_PLUGIN_COND_EQ,
>> + QEMU_PLUGIN_COND_NE,
>> + QEMU_PLUGIN_COND_LT,
>> + QEMU_PLUGIN_COND_LE,
>> + QEMU_PLUGIN_COND_GT,
>> + QEMU_PLUGIN_COND_GE,
>> +};
>> +
>> /**
>> * typedef qemu_plugin_vcpu_tb_trans_cb_t - translation callback
>> * @id: unique plugin id
>> @@ -301,6 +324,32 @@ void qemu_plugin_register_vcpu_tb_exec_cb(struct qemu_plugin_tb *tb,
>> enum qemu_plugin_cb_flags flags,
>> void *userdata);
>>
>> +/**
>> + * qemu_plugin_register_vcpu_tb_exec_cond_cb() - register conditional callback
>> + * @tb: the opaque qemu_plugin_tb handle for the translation
>> + * @cb: callback function
>> + * @cond: condition to enable callback
>> + * @entry: first operand for condition
>> + * @imm: second operand for condition
>> + * @flags: does the plugin read or write the CPU's registers?
>> + * @userdata: any plugin data to pass to the @cb?
>> + *
>> + * The @cb function is called when a translated unit executes if
>> + * entry @cond imm is true.
>> + * If condition is QEMU_PLUGIN_COND_ALWAYS, condition is never interpreted and
>> + * this function is equivalent to qemu_plugin_register_vcpu_tb_exec_cb.
>> + * If condition QEMU_PLUGIN_COND_NEVER, condition is never interpreted and
>> + * callback is never installed.
>> + */
>> +QEMU_PLUGIN_API
>> +void qemu_plugin_register_vcpu_tb_exec_cond_cb(struct qemu_plugin_tb *tb,
>> + qemu_plugin_vcpu_udata_cb_t cb,
>> + enum qemu_plugin_cb_flags flags,
>> + enum qemu_plugin_cond cond,
>> + qemu_plugin_u64 entry,
>
> Is this a fixed entry or part of a scoreboard?
>
entry is an entry of scoreboard (automatically associated to each vcpu
using vcpu_index) and can be modified by any other inline op, or
callback. @imm (next parameter) is fixed yes.
callback will be called only if entry <cond> imm true.
tests/plugin/inline.c has tests for this, and store operation.
> I'm trying to write a control flow plugin with a structure like:
>
> /* We use this to track the current execution state */
> typedef struct {
> /* address of start of block */
> uint64_t block_start;
> /* address of end of block */
> uint64_t block_end;
> /* address of last executed PC */
> uint64_t last_pc;
> } VCPUScoreBoard;
>
Seems ok.
> And I want to check to see if last_pc (set by STORE_U64 for each insn) == block_end
> (set at start of TB with what we know).
>
> Is this something I need to get with:
>
> last_pc = qemu_plugin_scoreboard_u64_in_struct(state, VCPUScoreBoard, last_pc);
>
Yes.
> ?
>
>> + uint64_t imm,
>> + void *userdata);
>> +
>> /**
>> * enum qemu_plugin_op - describes an inline op
>> *
>> @@ -344,6 +393,33 @@ void qemu_plugin_register_vcpu_insn_exec_cb(struct qemu_plugin_insn *insn,
>> enum qemu_plugin_cb_flags flags,
>> void *userdata);
>>
>> +/**
>> + * qemu_plugin_register_vcpu_insn_exec_cond_cb() - conditional insn execution cb
>> + * @insn: the opaque qemu_plugin_insn handle for an instruction
>> + * @cb: callback function
>> + * @flags: does the plugin read or write the CPU's registers?
>> + * @cond: condition to enable callback
>> + * @entry: first operand for condition
>> + * @imm: second operand for condition
>> + * @userdata: any plugin data to pass to the @cb?
>> + *
>> + * The @cb function is called when an instruction executes if
>> + * entry @cond imm is true.
>> + * If condition is QEMU_PLUGIN_COND_ALWAYS, condition is never interpreted and
>> + * this function is equivalent to qemu_plugin_register_vcpu_insn_exec_cb.
>> + * If condition QEMU_PLUGIN_COND_NEVER, condition is never interpreted and
>> + * callback is never installed.
>> + */
>> +QEMU_PLUGIN_API
>> +void qemu_plugin_register_vcpu_insn_exec_cond_cb(
>> + struct qemu_plugin_insn *insn,
>> + qemu_plugin_vcpu_udata_cb_t cb,
>> + enum qemu_plugin_cb_flags flags,
>> + enum qemu_plugin_cond cond,
>> + qemu_plugin_u64 entry,
>> + uint64_t imm,
>> + void *userdata);
>> +
>> /**
>> * qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu() - insn exec inline op
>> * @insn: the opaque qemu_plugin_insn handle for an instruction
>> diff --git a/plugins/plugin.h b/plugins/plugin.h
>> index 696b1fa38b0..118cc99bb2a 100644
>> --- a/plugins/plugin.h
>> +++ b/plugins/plugin.h
>> @@ -94,6 +94,14 @@ plugin_register_dyn_cb__udata(GArray **arr,
>> qemu_plugin_vcpu_udata_cb_t cb,
>> enum qemu_plugin_cb_flags flags, void *udata);
>>
>> +void
>> +plugin_register_dyn_cond_cb__udata(GArray **arr,
>> + qemu_plugin_vcpu_udata_cb_t cb,
>> + enum qemu_plugin_cb_flags flags,
>> + enum qemu_plugin_cond cond,
>> + qemu_plugin_u64 entry,
>> + uint64_t imm,
>> + void *udata);
>>
>> void plugin_register_vcpu_mem_cb(GArray **arr,
>> void *cb,
>> diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
>> index 02c894106e2..15762a26439 100644
>> --- a/accel/tcg/plugin-gen.c
>> +++ b/accel/tcg/plugin-gen.c
>> @@ -46,6 +46,7 @@
>> #include "qemu/plugin.h"
>> #include "cpu.h"
>> #include "tcg/tcg.h"
>> +#include "tcg/tcg-cond.h"
>> #include "tcg/tcg-internal.h"
>> #include "tcg/tcg-op.h"
>> #include "tcg/tcg-temp-internal.h"
>> @@ -82,6 +83,8 @@ enum plugin_gen_from {
>> enum plugin_gen_cb {
>> PLUGIN_GEN_CB_UDATA,
>> PLUGIN_GEN_CB_UDATA_R,
>> + PLUGIN_GEN_CB_COND_UDATA,
>> + PLUGIN_GEN_CB_COND_UDATA_R,
>> PLUGIN_GEN_CB_INLINE_ADD_U64,
>> PLUGIN_GEN_CB_INLINE_STORE_U64,
>> PLUGIN_GEN_CB_MEM,
>> @@ -119,16 +122,58 @@ static void gen_empty_udata_cb(void (*gen_helper)(TCGv_i32, TCGv_ptr))
>> tcg_temp_free_i32(cpu_index);
>> }
>>
>> +static void gen_empty_cond_udata_cb(void (*gen_helper)(TCGv_i32, TCGv_ptr))
>> +{
>> + TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
>> + TCGv_i32 cpu_offset = tcg_temp_ebb_new_i32();
>> + TCGv_ptr cpu_index_as_ptr = tcg_temp_ebb_new_ptr();
>> + TCGv_i64 val = tcg_temp_ebb_new_i64();
>> + TCGv_ptr ptr = tcg_temp_ebb_new_ptr();
>> + TCGv_ptr udata = tcg_temp_ebb_new_ptr();
>> + TCGLabel *after_cb = gen_new_label();
>> +
>> + tcg_gen_movi_ptr(udata, 0);
>> + tcg_gen_ld_i32(cpu_index, tcg_env,
>> + -offsetof(ArchCPU, env) + offsetof(CPUState, cpu_index));
>> + /* second operand will be replaced by immediate value */
>> + tcg_gen_mul_i32(cpu_offset, cpu_index, cpu_index);
>> + tcg_gen_ext_i32_ptr(cpu_index_as_ptr, cpu_offset);
>> + tcg_gen_movi_ptr(ptr, 0);
>> + tcg_gen_add_ptr(ptr, ptr, cpu_index_as_ptr);
>> + tcg_gen_ld_i64(val, ptr, 0);
>> +
>> + tcg_gen_brcondi_i64(TCG_COND_EQ, val, 0, after_cb);
>> + gen_helper(cpu_index, udata);
>> + gen_set_label(after_cb);
>> +
>> + tcg_temp_free_ptr(udata);
>> + tcg_temp_free_ptr(ptr);
>> + tcg_temp_free_i64(val);
>> + tcg_temp_free_ptr(cpu_index_as_ptr);
>> + tcg_temp_free_i32(cpu_offset);
>> + tcg_temp_free_i32(cpu_index);
>> +}
>> +
>> static void gen_empty_udata_cb_no_wg(void)
>> {
>> gen_empty_udata_cb(gen_helper_plugin_vcpu_udata_cb_no_wg);
>> }
>>
>> +static void gen_empty_cond_udata_cb_no_wg(void)
>> +{
>> + gen_empty_cond_udata_cb(gen_helper_plugin_vcpu_udata_cb_no_wg);
>> +}
>> +
>> static void gen_empty_udata_cb_no_rwg(void)
>> {
>> gen_empty_udata_cb(gen_helper_plugin_vcpu_udata_cb_no_rwg);
>> }
>>
>> +static void gen_empty_cond_udata_cb_no_rwg(void)
>> +{
>> + gen_empty_cond_udata_cb(gen_helper_plugin_vcpu_udata_cb_no_rwg);
>> +}
>> +
>> static void gen_empty_inline_cb_add_u64(void)
>> {
>> TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
>> @@ -248,6 +293,10 @@ static void plugin_gen_empty_callback(enum plugin_gen_from from)
>> gen_empty_inline_cb_store_u64);
>> gen_wrapped(from, PLUGIN_GEN_CB_UDATA, gen_empty_udata_cb_no_rwg);
>> gen_wrapped(from, PLUGIN_GEN_CB_UDATA_R, gen_empty_udata_cb_no_wg);
>> + gen_wrapped(from, PLUGIN_GEN_CB_COND_UDATA,
>> + gen_empty_cond_udata_cb_no_rwg);
>> + gen_wrapped(from, PLUGIN_GEN_CB_COND_UDATA_R,
>> + gen_empty_cond_udata_cb_no_wg);
>> break;
>> default:
>> g_assert_not_reached();
>> @@ -456,6 +505,29 @@ static TCGOp *copy_call(TCGOp **begin_op, TCGOp *op, void *func, int *cb_idx)
>> return op;
>> }
>>
>> +static TCGOp *copy_brcondi_i64(TCGOp **begin_op, TCGOp *op, TCGCond cond,
>> + uint64_t imm)
>> +{
>> + if (TCG_TARGET_REG_BITS == 32) {
>> + op = copy_op(begin_op, op, INDEX_op_brcond2_i32);
>> + /* low(arg1), high(arg1), 32(arg2), 32(arg2 >> 32), cond, label */
>> + op->args[2] = tcgv_i32_arg(tcg_constant_i32(imm));
>> + op->args[3] = tcgv_i32_arg(tcg_constant_i32(imm >> 32));
>> + op->args[4] = cond;
>> + } else {
>> + op = copy_op(begin_op, op, INDEX_op_brcond_i64);
>> + /* arg1, arg2, cond, label */
>> + op->args[1] = tcgv_i64_arg(tcg_constant_i64(imm));
>> + op->args[2] = cond;
>> + }
>> + return op;
>> +}
>> +
>> +static TCGOp *copy_set_label(TCGOp **begin_op, TCGOp *op)
>> +{
>> + return copy_op(begin_op, op, INDEX_op_set_label);
>> +}
>> +
>> /*
>> * When we append/replace ops here we are sensitive to changing patterns of
>> * TCGOps generated by the tcg_gen_FOO calls when we generated the
>> @@ -482,6 +554,50 @@ static TCGOp *append_udata_cb(const struct qemu_plugin_dyn_cb *cb,
>> return op;
>> }
>>
>> +static TCGCond plugin_cond_to_tcgcond(enum qemu_plugin_cond cond)
>> +{
>> + switch (cond) {
>> + case QEMU_PLUGIN_COND_EQ:
>> + return TCG_COND_EQ;
>> + case QEMU_PLUGIN_COND_NE:
>> + return TCG_COND_NE;
>> + case QEMU_PLUGIN_COND_LT:
>> + return TCG_COND_LTU;
>> + case QEMU_PLUGIN_COND_LE:
>> + return TCG_COND_LEU;
>> + case QEMU_PLUGIN_COND_GT:
>> + return TCG_COND_GTU;
>> + case QEMU_PLUGIN_COND_GE:
>> + return TCG_COND_GEU;
>> + default:
>> + /* ALWAYS and NEVER conditions should never reach */
>> + g_assert_not_reached();
>> + }
>> +}
>> +
>> +static TCGOp *append_cond_udata_cb(const struct qemu_plugin_dyn_cb *cb,
>> + TCGOp *begin_op, TCGOp *op, int *cb_idx)
>> +{
>> + char *ptr = cb->cond_cb.entry.score->data->data;
>> + size_t elem_size = g_array_get_element_size(
>> + cb->cond_cb.entry.score->data);
>> + size_t offset = cb->cond_cb.entry.offset;
>> + /* Condition should be negated, as calling the cb is the "else" path */
>> + TCGCond cond = tcg_invert_cond(plugin_cond_to_tcgcond(cb->cond_cb.cond));
>> +
>> + op = copy_const_ptr(&begin_op, op, ptr);
>> + op = copy_ld_i32(&begin_op, op);
>> + op = copy_mul_i32(&begin_op, op, elem_size);
>> + op = copy_ext_i32_ptr(&begin_op, op);
>> + op = copy_const_ptr(&begin_op, op, ptr + offset);
>> + op = copy_add_ptr(&begin_op, op);
>> + op = copy_ld_i64(&begin_op, op);
>> + op = copy_brcondi_i64(&begin_op, op, cond, cb->cond_cb.imm);
>> + op = copy_call(&begin_op, op, cb->f.vcpu_udata, cb_idx);
>> + op = copy_set_label(&begin_op, op);
>> + return op;
>> +}
>> +
>> static TCGOp *append_inline_cb_add_u64(const struct qemu_plugin_dyn_cb *cb,
>> TCGOp *begin_op, TCGOp *op,
>> int *unused)
>> @@ -601,6 +717,12 @@ inject_udata_cb(const GArray *cbs, TCGOp *begin_op)
>> inject_cb_type(cbs, begin_op, append_udata_cb, op_ok);
>> }
>>
>> +static void
>> +inject_cond_udata_cb(const GArray *cbs, TCGOp *begin_op)
>> +{
>> + inject_cb_type(cbs, begin_op, append_cond_udata_cb, op_ok);
>> +}
>> +
>> static void
>> inject_inline_cb_add_u64(const GArray *cbs, TCGOp *begin_op, op_ok_fn ok)
>> {
>> @@ -654,7 +776,7 @@ static void inject_mem_enable_helper(struct qemu_plugin_tb *ptb,
>> struct qemu_plugin_insn *plugin_insn,
>> TCGOp *begin_op)
>> {
>> - GArray *cbs[3];
>> + GArray *cbs[4];
>> GArray *arr;
>> size_t n_cbs, i;
>>
>> @@ -662,6 +784,7 @@ static void inject_mem_enable_helper(struct qemu_plugin_tb *ptb,
>> cbs[0] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_INLINE_ADD_U64];
>> cbs[1] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_INLINE_STORE_U64];
>> cbs[2] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_REGULAR];
>> + cbs[3] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_COND];
>>
>> n_cbs = 0;
>> for (i = 0; i < ARRAY_SIZE(cbs); i++) {
>> @@ -727,6 +850,18 @@ static void plugin_gen_tb_udata_r(const struct qemu_plugin_tb *ptb,
>> inject_udata_cb(ptb->cbs[PLUGIN_CB_REGULAR_R], begin_op);
>> }
>>
>> +static void plugin_gen_tb_cond_udata(const struct qemu_plugin_tb *ptb,
>> + TCGOp *begin_op)
>> +{
>> + inject_cond_udata_cb(ptb->cbs[PLUGIN_CB_COND], begin_op);
>> +}
>> +
>> +static void plugin_gen_tb_cond_udata_r(const struct qemu_plugin_tb *ptb,
>> + TCGOp *begin_op)
>> +{
>> + inject_cond_udata_cb(ptb->cbs[PLUGIN_CB_COND_R], begin_op);
>> +}
>> +
>> static void plugin_gen_tb_inline_add_u64(const struct qemu_plugin_tb *ptb,
>> TCGOp *begin_op)
>> {
>> @@ -745,7 +880,6 @@ static void plugin_gen_insn_udata(const struct qemu_plugin_tb *ptb,
>> TCGOp *begin_op, int insn_idx)
>> {
>> struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
>> -
>> inject_udata_cb(insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_REGULAR], begin_op);
>> }
>>
>> @@ -753,10 +887,23 @@ static void plugin_gen_insn_udata_r(const struct qemu_plugin_tb *ptb,
>> TCGOp *begin_op, int insn_idx)
>> {
>> struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
>> -
>> inject_udata_cb(insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_REGULAR_R], begin_op);
>> }
>>
>> +static void plugin_gen_insn_cond_udata(const struct qemu_plugin_tb *ptb,
>> + TCGOp *begin_op, int insn_idx)
>> +{
>> + struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
>> + inject_cond_udata_cb(insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_COND], begin_op);
>> +}
>> +
>> +static void plugin_gen_insn_cond_udata_r(const struct qemu_plugin_tb *ptb,
>> + TCGOp *begin_op, int insn_idx)
>> +{
>> + struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
>> + inject_cond_udata_cb(insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_COND_R], begin_op);
>> +}
>> +
>> static void plugin_gen_insn_inline_add_u64(const struct qemu_plugin_tb *ptb,
>> TCGOp *begin_op, int insn_idx)
>> {
>> @@ -843,6 +990,15 @@ static void pr_ops(void)
>> case PLUGIN_GEN_CB_UDATA:
>> type = "udata";
>> break;
>> + case PLUGIN_GEN_CB_UDATA_R:
>> + type = "udata (read)";
>> + break;
>> + case PLUGIN_GEN_CB_COND_UDATA:
>> + type = "cond udata";
>> + break;
>> + case PLUGIN_GEN_CB_COND_UDATA_R:
>> + type = "cond udata (read)";
>> + break;
>> case PLUGIN_GEN_CB_INLINE_ADD_U64:
>> type = "inline add u64";
>> break;
>> @@ -897,6 +1053,12 @@ static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
>> case PLUGIN_GEN_CB_UDATA_R:
>> plugin_gen_tb_udata_r(plugin_tb, op);
>> break;
>> + case PLUGIN_GEN_CB_COND_UDATA:
>> + plugin_gen_tb_cond_udata(plugin_tb, op);
>> + break;
>> + case PLUGIN_GEN_CB_COND_UDATA_R:
>> + plugin_gen_tb_cond_udata_r(plugin_tb, op);
>> + break;
>> case PLUGIN_GEN_CB_INLINE_ADD_U64:
>> plugin_gen_tb_inline_add_u64(plugin_tb, op);
>> break;
>> @@ -919,6 +1081,12 @@ static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
>> case PLUGIN_GEN_CB_UDATA_R:
>> plugin_gen_insn_udata_r(plugin_tb, op, insn_idx);
>> break;
>> + case PLUGIN_GEN_CB_COND_UDATA:
>> + plugin_gen_insn_cond_udata(plugin_tb, op, insn_idx);
>> + break;
>> + case PLUGIN_GEN_CB_COND_UDATA_R:
>> + plugin_gen_insn_cond_udata_r(plugin_tb, op, insn_idx);
>> + break;
>> case PLUGIN_GEN_CB_INLINE_ADD_U64:
>> plugin_gen_insn_inline_add_u64(plugin_tb, op, insn_idx);
>> break;
>> diff --git a/plugins/api.c b/plugins/api.c
>> index 4a033c16f83..f24d68d7a68 100644
>> --- a/plugins/api.c
>> +++ b/plugins/api.c
>> @@ -113,6 +113,31 @@ void qemu_plugin_register_vcpu_tb_exec_cb(struct qemu_plugin_tb *tb,
>> }
>> }
>>
>> +void qemu_plugin_register_vcpu_tb_exec_cond_cb(struct qemu_plugin_tb *tb,
>> + qemu_plugin_vcpu_udata_cb_t cb,
>> + enum qemu_plugin_cb_flags flags,
>> + enum qemu_plugin_cond cond,
>> + qemu_plugin_u64 entry,
>> + uint64_t imm,
>> + void *udata)
>> +{
>> + if (cond == QEMU_PLUGIN_COND_NEVER || tb->mem_only) {
>> + return;
>> + }
>> + if (cond == QEMU_PLUGIN_COND_ALWAYS) {
>> + qemu_plugin_register_vcpu_tb_exec_cb(tb, cb, flags, udata);
>> + return;
>> + }
>> + int index = flags == QEMU_PLUGIN_CB_R_REGS ||
>> + flags == QEMU_PLUGIN_CB_RW_REGS ?
>> + PLUGIN_CB_COND_R : PLUGIN_CB_COND;
>> +
>> + plugin_register_dyn_cond_cb__udata(&tb->cbs[index],
>> + cb, flags,
>> + cond, entry, imm,
>> + udata);
>> +}
>> +
>> void qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
>> struct qemu_plugin_tb *tb,
>> enum qemu_plugin_op op,
>> @@ -141,6 +166,32 @@ void qemu_plugin_register_vcpu_insn_exec_cb(struct qemu_plugin_insn *insn,
>> }
>> }
>>
>> +void qemu_plugin_register_vcpu_insn_exec_cond_cb(
>> + struct qemu_plugin_insn *insn,
>> + qemu_plugin_vcpu_udata_cb_t cb,
>> + enum qemu_plugin_cb_flags flags,
>> + enum qemu_plugin_cond cond,
>> + qemu_plugin_u64 entry,
>> + uint64_t imm,
>> + void *udata)
>> +{
>> + if (cond == QEMU_PLUGIN_COND_NEVER || insn->mem_only) {
>> + return;
>> + }
>> + if (cond == QEMU_PLUGIN_COND_ALWAYS) {
>> + qemu_plugin_register_vcpu_insn_exec_cb(insn, cb, flags, udata);
>> + return;
>> + }
>> + int index = flags == QEMU_PLUGIN_CB_R_REGS ||
>> + flags == QEMU_PLUGIN_CB_RW_REGS ?
>> + PLUGIN_CB_COND_R : PLUGIN_CB_COND;
>> +
>> + plugin_register_dyn_cond_cb__udata(&insn->cbs[PLUGIN_CB_INSN][index],
>> + cb, flags,
>> + cond, entry, imm,
>> + udata);
>> +}
>> +
>> void qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
>> struct qemu_plugin_insn *insn,
>> enum qemu_plugin_op op,
>> diff --git a/plugins/core.c b/plugins/core.c
>> index 11f72594229..82d5f17438e 100644
>> --- a/plugins/core.c
>> +++ b/plugins/core.c
>> @@ -347,6 +347,25 @@ void plugin_register_dyn_cb__udata(GArray **arr,
>> dyn_cb->type = PLUGIN_CB_REGULAR;
>> }
>>
>> +void plugin_register_dyn_cond_cb__udata(GArray **arr,
>> + qemu_plugin_vcpu_udata_cb_t cb,
>> + enum qemu_plugin_cb_flags flags,
>> + enum qemu_plugin_cond cond,
>> + qemu_plugin_u64 entry,
>> + uint64_t imm,
>> + void *udata)
>> +{
>> + struct qemu_plugin_dyn_cb *dyn_cb = plugin_get_dyn_cb(arr);
>> +
>> + dyn_cb->userp = udata;
>> + /* Note flags are discarded as unused. */
>> + dyn_cb->f.vcpu_udata = cb;
>> + dyn_cb->type = PLUGIN_CB_COND;
>> + dyn_cb->cond_cb.cond = cond;
>> + dyn_cb->cond_cb.entry = entry;
>> + dyn_cb->cond_cb.imm = imm;
>> +}
>> +
>> void plugin_register_vcpu_mem_cb(GArray **arr,
>> void *cb,
>> enum qemu_plugin_cb_flags flags,
>> diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols
>> index a9fac056c7f..aa0a77a319f 100644
>> --- a/plugins/qemu-plugins.symbols
>> +++ b/plugins/qemu-plugins.symbols
>> @@ -27,6 +27,7 @@
>> qemu_plugin_register_vcpu_idle_cb;
>> qemu_plugin_register_vcpu_init_cb;
>> qemu_plugin_register_vcpu_insn_exec_cb;
>> + qemu_plugin_register_vcpu_insn_exec_cond_cb;
>> qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu;
>> qemu_plugin_register_vcpu_mem_cb;
>> qemu_plugin_register_vcpu_mem_inline_per_vcpu;
>> @@ -34,6 +35,7 @@
>> qemu_plugin_register_vcpu_syscall_cb;
>> qemu_plugin_register_vcpu_syscall_ret_cb;
>> qemu_plugin_register_vcpu_tb_exec_cb;
>> + qemu_plugin_register_vcpu_tb_exec_cond_cb;
>> qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu;
>> qemu_plugin_register_vcpu_tb_trans_cb;
>> qemu_plugin_reset;
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5] plugins: conditional callbacks
2024-03-11 15:43 ` Alex Bennée
@ 2024-03-12 6:03 ` Pierrick Bouvier
2024-03-12 7:37 ` Pierrick Bouvier
0 siblings, 1 reply; 14+ messages in thread
From: Pierrick Bouvier @ 2024-03-12 6:03 UTC (permalink / raw)
To: Alex Bennée
Cc: qemu-devel, Mahmoud Mandour, Paolo Bonzini, Alexandre Iooss,
Richard Henderson
On 3/11/24 19:43, Alex Bennée wrote:
> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
>
>> Extend plugins API to support callback called with a given criteria
>> (evaluated inline).
>>
>> Added functions:
>> - qemu_plugin_register_vcpu_tb_exec_cond_cb
>> - qemu_plugin_register_vcpu_insn_exec_cond_cb
>>
>> They expect as parameter a condition, a qemu_plugin_u64_t (op1) and an
>> immediate (op2). Callback is called if op1 |cond| op2 is true.
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> <snip>
>>
>> +static TCGCond plugin_cond_to_tcgcond(enum qemu_plugin_cond cond)
>> +{
>> + switch (cond) {
>> + case QEMU_PLUGIN_COND_EQ:
>> + return TCG_COND_EQ;
>> + case QEMU_PLUGIN_COND_NE:
>> + return TCG_COND_NE;
>> + case QEMU_PLUGIN_COND_LT:
>> + return TCG_COND_LTU;
>> + case QEMU_PLUGIN_COND_LE:
>> + return TCG_COND_LEU;
>> + case QEMU_PLUGIN_COND_GT:
>> + return TCG_COND_GTU;
>> + case QEMU_PLUGIN_COND_GE:
>> + return TCG_COND_GEU;
>> + default:
>> + /* ALWAYS and NEVER conditions should never reach */
>> + g_assert_not_reached();
>> + }
>> +}
>> +
>> +static TCGOp *append_cond_udata_cb(const struct qemu_plugin_dyn_cb *cb,
>> + TCGOp *begin_op, TCGOp *op, int *cb_idx)
>> +{
>> + char *ptr = cb->cond_cb.entry.score->data->data;
>> + size_t elem_size = g_array_get_element_size(
>> + cb->cond_cb.entry.score->data);
>> + size_t offset = cb->cond_cb.entry.offset;
>> + /* Condition should be negated, as calling the cb is the "else" path */
>> + TCGCond cond = tcg_invert_cond(plugin_cond_to_tcgcond(cb->cond_cb.cond));
>> +
>> + op = copy_const_ptr(&begin_op, op, ptr);
>> + op = copy_ld_i32(&begin_op, op);
>> + op = copy_mul_i32(&begin_op, op, elem_size);
>> + op = copy_ext_i32_ptr(&begin_op, op);
>> + op = copy_const_ptr(&begin_op, op, ptr + offset);
>> + op = copy_add_ptr(&begin_op, op);
>> + op = copy_ld_i64(&begin_op, op);
>> + op = copy_brcondi_i64(&begin_op, op, cond, cb->cond_cb.imm);
>> + op = copy_call(&begin_op, op, cb->f.vcpu_udata, cb_idx);
>> + op = copy_set_label(&begin_op, op);
>> + return op;
>
> I think we are missing something here to ensure that udata is set
> correctly for the callback, see my RFC:
>
> Subject: [RFC PATCH] contrib/plugins: control flow plugin (WIP!)
> Date: Mon, 11 Mar 2024 15:34:32 +0000
> Message-Id: <20240311153432.1395190-1-alex.bennee@linaro.org>
>
> which is seeing the same value every time in the callback.
>
I'm trying to reproduce and will answer on this thread.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5] plugins: conditional callbacks
2024-03-12 6:03 ` Pierrick Bouvier
@ 2024-03-12 7:37 ` Pierrick Bouvier
0 siblings, 0 replies; 14+ messages in thread
From: Pierrick Bouvier @ 2024-03-12 7:37 UTC (permalink / raw)
To: Alex Bennée
Cc: qemu-devel, Mahmoud Mandour, Paolo Bonzini, Alexandre Iooss,
Richard Henderson
On 3/12/24 10:03, Pierrick Bouvier wrote:
> On 3/11/24 19:43, Alex Bennée wrote:
>> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
>>
>>> Extend plugins API to support callback called with a given criteria
>>> (evaluated inline).
>>>
>>> Added functions:
>>> - qemu_plugin_register_vcpu_tb_exec_cond_cb
>>> - qemu_plugin_register_vcpu_insn_exec_cond_cb
>>>
>>> They expect as parameter a condition, a qemu_plugin_u64_t (op1) and an
>>> immediate (op2). Callback is called if op1 |cond| op2 is true.
>>>
>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> <snip>
>>>
>>> +static TCGCond plugin_cond_to_tcgcond(enum qemu_plugin_cond cond)
>>> +{
>>> + switch (cond) {
>>> + case QEMU_PLUGIN_COND_EQ:
>>> + return TCG_COND_EQ;
>>> + case QEMU_PLUGIN_COND_NE:
>>> + return TCG_COND_NE;
>>> + case QEMU_PLUGIN_COND_LT:
>>> + return TCG_COND_LTU;
>>> + case QEMU_PLUGIN_COND_LE:
>>> + return TCG_COND_LEU;
>>> + case QEMU_PLUGIN_COND_GT:
>>> + return TCG_COND_GTU;
>>> + case QEMU_PLUGIN_COND_GE:
>>> + return TCG_COND_GEU;
>>> + default:
>>> + /* ALWAYS and NEVER conditions should never reach */
>>> + g_assert_not_reached();
>>> + }
>>> +}
>>> +
>>> +static TCGOp *append_cond_udata_cb(const struct qemu_plugin_dyn_cb *cb,
>>> + TCGOp *begin_op, TCGOp *op, int *cb_idx)
>>> +{
>>> + char *ptr = cb->cond_cb.entry.score->data->data;
>>> + size_t elem_size = g_array_get_element_size(
>>> + cb->cond_cb.entry.score->data);
>>> + size_t offset = cb->cond_cb.entry.offset;
>>> + /* Condition should be negated, as calling the cb is the "else" path */
>>> + TCGCond cond = tcg_invert_cond(plugin_cond_to_tcgcond(cb->cond_cb.cond));
>>> +
>>> + op = copy_const_ptr(&begin_op, op, ptr);
This line was wrong, and cb->userp should be copied instead.
I'll fix this, add a test specifically checking udata for conditional
callback and resend the series.
>>> + op = copy_ld_i32(&begin_op, op);
>>> + op = copy_mul_i32(&begin_op, op, elem_size);
>>> + op = copy_ext_i32_ptr(&begin_op, op);
>>> + op = copy_const_ptr(&begin_op, op, ptr + offset);
>>> + op = copy_add_ptr(&begin_op, op);
>>> + op = copy_ld_i64(&begin_op, op);
>>> + op = copy_brcondi_i64(&begin_op, op, cond, cb->cond_cb.imm);
>>> + op = copy_call(&begin_op, op, cb->f.vcpu_udata, cb_idx);
>>> + op = copy_set_label(&begin_op, op);
>>> + return op;
>>
>> I think we are missing something here to ensure that udata is set
>> correctly for the callback, see my RFC:
>>
>> Subject: [RFC PATCH] contrib/plugins: control flow plugin (WIP!)
>> Date: Mon, 11 Mar 2024 15:34:32 +0000
>> Message-Id: <20240311153432.1395190-1-alex.bennee@linaro.org>
>>
>> which is seeing the same value every time in the callback.
>>
>
> I'm trying to reproduce and will answer on this thread.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5] plugins: conditional callbacks
2024-03-12 6:03 ` Pierrick Bouvier
@ 2024-03-12 15:04 ` Alex Bennée
2024-03-12 16:04 ` Pierrick Bouvier
0 siblings, 1 reply; 14+ messages in thread
From: Alex Bennée @ 2024-03-12 15:04 UTC (permalink / raw)
To: Pierrick Bouvier
Cc: qemu-devel, Mahmoud Mandour, Paolo Bonzini, Alexandre Iooss,
Richard Henderson
Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
> On 3/11/24 14:08, Alex Bennée wrote:
>> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
>>
>>> Extend plugins API to support callback called with a given criteria
>>> (evaluated inline).
>>>
>>> Added functions:
>>> - qemu_plugin_register_vcpu_tb_exec_cond_cb
>>> - qemu_plugin_register_vcpu_insn_exec_cond_cb
>>>
>>> They expect as parameter a condition, a qemu_plugin_u64_t (op1) and an
>>> immediate (op2). Callback is called if op1 |cond| op2 is true.
>>>
>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>> ---
>>> include/qemu/plugin.h | 7 ++
>>> include/qemu/qemu-plugin.h | 76 +++++++++++++++
>>> plugins/plugin.h | 8 ++
>>> accel/tcg/plugin-gen.c | 174 ++++++++++++++++++++++++++++++++++-
>>> plugins/api.c | 51 ++++++++++
>>> plugins/core.c | 19 ++++
>>> plugins/qemu-plugins.symbols | 2 +
>>> 7 files changed, 334 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
>>> index d92d64744e6..056102b2361 100644
>>> --- a/include/qemu/plugin.h
>>> +++ b/include/qemu/plugin.h
>>> @@ -74,6 +74,8 @@ enum plugin_dyn_cb_type {
>>> enum plugin_dyn_cb_subtype {
>>> PLUGIN_CB_REGULAR,
>>> PLUGIN_CB_REGULAR_R,
>>> + PLUGIN_CB_COND,
>>> + PLUGIN_CB_COND_R,
>>> PLUGIN_CB_INLINE_ADD_U64,
>>> PLUGIN_CB_INLINE_STORE_U64,
>>> PLUGIN_N_CB_SUBTYPES,
>>> @@ -97,6 +99,11 @@ struct qemu_plugin_dyn_cb {
>>> enum qemu_plugin_op op;
>>> uint64_t imm;
>>> } inline_insn;
>>> + struct {
>>> + qemu_plugin_u64 entry;
>>> + enum qemu_plugin_cond cond;
>>> + uint64_t imm;
>>> + } cond_cb;
>>> };
>>> };
>>> diff --git a/include/qemu/qemu-plugin.h
>>> b/include/qemu/qemu-plugin.h
>>> index c5cac897a0b..337de25ece7 100644
>>> --- a/include/qemu/qemu-plugin.h
>>> +++ b/include/qemu/qemu-plugin.h
>>> @@ -262,6 +262,29 @@ enum qemu_plugin_mem_rw {
>>> QEMU_PLUGIN_MEM_RW,
>>> };
>>> +/**
>>> + * enum qemu_plugin_cond - condition to enable callback
>>> + *
>>> + * @QEMU_PLUGIN_COND_NEVER: false
>>> + * @QEMU_PLUGIN_COND_ALWAYS: true
>>> + * @QEMU_PLUGIN_COND_EQ: is equal?
>>> + * @QEMU_PLUGIN_COND_NE: is not equal?
>>> + * @QEMU_PLUGIN_COND_LT: is less than?
>>> + * @QEMU_PLUGIN_COND_LE: is less than or equal?
>>> + * @QEMU_PLUGIN_COND_GT: is greater than?
>>> + * @QEMU_PLUGIN_COND_GE: is greater than or equal?
>>> + */
>>> +enum qemu_plugin_cond {
>>> + QEMU_PLUGIN_COND_NEVER,
>>> + QEMU_PLUGIN_COND_ALWAYS,
>>> + QEMU_PLUGIN_COND_EQ,
>>> + QEMU_PLUGIN_COND_NE,
>>> + QEMU_PLUGIN_COND_LT,
>>> + QEMU_PLUGIN_COND_LE,
>>> + QEMU_PLUGIN_COND_GT,
>>> + QEMU_PLUGIN_COND_GE,
>>> +};
>>> +
>>> /**
>>> * typedef qemu_plugin_vcpu_tb_trans_cb_t - translation callback
>>> * @id: unique plugin id
>>> @@ -301,6 +324,32 @@ void qemu_plugin_register_vcpu_tb_exec_cb(struct qemu_plugin_tb *tb,
>>> enum qemu_plugin_cb_flags flags,
>>> void *userdata);
>>> +/**
>>> + * qemu_plugin_register_vcpu_tb_exec_cond_cb() - register conditional callback
>>> + * @tb: the opaque qemu_plugin_tb handle for the translation
>>> + * @cb: callback function
>>> + * @cond: condition to enable callback
>>> + * @entry: first operand for condition
>>> + * @imm: second operand for condition
>>> + * @flags: does the plugin read or write the CPU's registers?
>>> + * @userdata: any plugin data to pass to the @cb?
>>> + *
>>> + * The @cb function is called when a translated unit executes if
>>> + * entry @cond imm is true.
>>> + * If condition is QEMU_PLUGIN_COND_ALWAYS, condition is never interpreted and
>>> + * this function is equivalent to qemu_plugin_register_vcpu_tb_exec_cb.
>>> + * If condition QEMU_PLUGIN_COND_NEVER, condition is never interpreted and
>>> + * callback is never installed.
>>> + */
>>> +QEMU_PLUGIN_API
>>> +void qemu_plugin_register_vcpu_tb_exec_cond_cb(struct qemu_plugin_tb *tb,
>>> + qemu_plugin_vcpu_udata_cb_t cb,
>>> + enum qemu_plugin_cb_flags flags,
>>> + enum qemu_plugin_cond cond,
>>> + qemu_plugin_u64 entry,
>> Is this a fixed entry or part of a scoreboard?
>>
>
> entry is an entry of scoreboard (automatically associated to each vcpu
> using vcpu_index) and can be modified by any other inline op, or
> callback. @imm (next parameter) is fixed yes.
>
> callback will be called only if entry <cond> imm true.
I wonder if having an alternate form for comparing two scoreboard
entries would be useful?
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5] plugins: conditional callbacks
2024-03-12 15:04 ` Alex Bennée
@ 2024-03-12 16:04 ` Pierrick Bouvier
0 siblings, 0 replies; 14+ messages in thread
From: Pierrick Bouvier @ 2024-03-12 16:04 UTC (permalink / raw)
To: Alex Bennée
Cc: qemu-devel, Mahmoud Mandour, Paolo Bonzini, Alexandre Iooss,
Richard Henderson
On 3/12/24 19:04, Alex Bennée wrote:
> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
>
>> On 3/11/24 14:08, Alex Bennée wrote:
>>> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
>>>
>>>> Extend plugins API to support callback called with a given criteria
>>>> (evaluated inline).
>>>>
>>>> Added functions:
>>>> - qemu_plugin_register_vcpu_tb_exec_cond_cb
>>>> - qemu_plugin_register_vcpu_insn_exec_cond_cb
>>>>
>>>> They expect as parameter a condition, a qemu_plugin_u64_t (op1) and an
>>>> immediate (op2). Callback is called if op1 |cond| op2 is true.
>>>>
>>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>> ---
>>>> include/qemu/plugin.h | 7 ++
>>>> include/qemu/qemu-plugin.h | 76 +++++++++++++++
>>>> plugins/plugin.h | 8 ++
>>>> accel/tcg/plugin-gen.c | 174 ++++++++++++++++++++++++++++++++++-
>>>> plugins/api.c | 51 ++++++++++
>>>> plugins/core.c | 19 ++++
>>>> plugins/qemu-plugins.symbols | 2 +
>>>> 7 files changed, 334 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
>>>> index d92d64744e6..056102b2361 100644
>>>> --- a/include/qemu/plugin.h
>>>> +++ b/include/qemu/plugin.h
>>>> @@ -74,6 +74,8 @@ enum plugin_dyn_cb_type {
>>>> enum plugin_dyn_cb_subtype {
>>>> PLUGIN_CB_REGULAR,
>>>> PLUGIN_CB_REGULAR_R,
>>>> + PLUGIN_CB_COND,
>>>> + PLUGIN_CB_COND_R,
>>>> PLUGIN_CB_INLINE_ADD_U64,
>>>> PLUGIN_CB_INLINE_STORE_U64,
>>>> PLUGIN_N_CB_SUBTYPES,
>>>> @@ -97,6 +99,11 @@ struct qemu_plugin_dyn_cb {
>>>> enum qemu_plugin_op op;
>>>> uint64_t imm;
>>>> } inline_insn;
>>>> + struct {
>>>> + qemu_plugin_u64 entry;
>>>> + enum qemu_plugin_cond cond;
>>>> + uint64_t imm;
>>>> + } cond_cb;
>>>> };
>>>> };
>>>> diff --git a/include/qemu/qemu-plugin.h
>>>> b/include/qemu/qemu-plugin.h
>>>> index c5cac897a0b..337de25ece7 100644
>>>> --- a/include/qemu/qemu-plugin.h
>>>> +++ b/include/qemu/qemu-plugin.h
>>>> @@ -262,6 +262,29 @@ enum qemu_plugin_mem_rw {
>>>> QEMU_PLUGIN_MEM_RW,
>>>> };
>>>> +/**
>>>> + * enum qemu_plugin_cond - condition to enable callback
>>>> + *
>>>> + * @QEMU_PLUGIN_COND_NEVER: false
>>>> + * @QEMU_PLUGIN_COND_ALWAYS: true
>>>> + * @QEMU_PLUGIN_COND_EQ: is equal?
>>>> + * @QEMU_PLUGIN_COND_NE: is not equal?
>>>> + * @QEMU_PLUGIN_COND_LT: is less than?
>>>> + * @QEMU_PLUGIN_COND_LE: is less than or equal?
>>>> + * @QEMU_PLUGIN_COND_GT: is greater than?
>>>> + * @QEMU_PLUGIN_COND_GE: is greater than or equal?
>>>> + */
>>>> +enum qemu_plugin_cond {
>>>> + QEMU_PLUGIN_COND_NEVER,
>>>> + QEMU_PLUGIN_COND_ALWAYS,
>>>> + QEMU_PLUGIN_COND_EQ,
>>>> + QEMU_PLUGIN_COND_NE,
>>>> + QEMU_PLUGIN_COND_LT,
>>>> + QEMU_PLUGIN_COND_LE,
>>>> + QEMU_PLUGIN_COND_GT,
>>>> + QEMU_PLUGIN_COND_GE,
>>>> +};
>>>> +
>>>> /**
>>>> * typedef qemu_plugin_vcpu_tb_trans_cb_t - translation callback
>>>> * @id: unique plugin id
>>>> @@ -301,6 +324,32 @@ void qemu_plugin_register_vcpu_tb_exec_cb(struct qemu_plugin_tb *tb,
>>>> enum qemu_plugin_cb_flags flags,
>>>> void *userdata);
>>>> +/**
>>>> + * qemu_plugin_register_vcpu_tb_exec_cond_cb() - register conditional callback
>>>> + * @tb: the opaque qemu_plugin_tb handle for the translation
>>>> + * @cb: callback function
>>>> + * @cond: condition to enable callback
>>>> + * @entry: first operand for condition
>>>> + * @imm: second operand for condition
>>>> + * @flags: does the plugin read or write the CPU's registers?
>>>> + * @userdata: any plugin data to pass to the @cb?
>>>> + *
>>>> + * The @cb function is called when a translated unit executes if
>>>> + * entry @cond imm is true.
>>>> + * If condition is QEMU_PLUGIN_COND_ALWAYS, condition is never interpreted and
>>>> + * this function is equivalent to qemu_plugin_register_vcpu_tb_exec_cb.
>>>> + * If condition QEMU_PLUGIN_COND_NEVER, condition is never interpreted and
>>>> + * callback is never installed.
>>>> + */
>>>> +QEMU_PLUGIN_API
>>>> +void qemu_plugin_register_vcpu_tb_exec_cond_cb(struct qemu_plugin_tb *tb,
>>>> + qemu_plugin_vcpu_udata_cb_t cb,
>>>> + enum qemu_plugin_cb_flags flags,
>>>> + enum qemu_plugin_cond cond,
>>>> + qemu_plugin_u64 entry,
>>> Is this a fixed entry or part of a scoreboard?
>>>
>>
>> entry is an entry of scoreboard (automatically associated to each vcpu
>> using vcpu_index) and can be modified by any other inline op, or
>> callback. @imm (next parameter) is fixed yes.
>>
>> callback will be called only if entry <cond> imm true.
>
> I wonder if having an alternate form for comparing two scoreboard
> entries would be useful?
>
We can always add a new API for that in the future if a specific need is
identified. In our current use cases, this need was not revealed.
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2024-03-12 16:05 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-29 5:53 [PATCH 0/5] TCG plugins new inline operations Pierrick Bouvier
2024-02-29 5:53 ` [PATCH 1/5] plugins: prepare introduction of new inline ops Pierrick Bouvier
2024-02-29 5:53 ` [PATCH 2/5] plugins: add new inline op STORE_U64 Pierrick Bouvier
2024-02-29 5:53 ` [PATCH 3/5] tests/plugin/inline: add test for STORE_U64 inline op Pierrick Bouvier
2024-02-29 5:53 ` [PATCH 4/5] plugins: conditional callbacks Pierrick Bouvier
2024-03-11 10:08 ` Alex Bennée
2024-03-12 6:03 ` Pierrick Bouvier
2024-03-12 15:04 ` Alex Bennée
2024-03-12 16:04 ` Pierrick Bouvier
2024-03-11 15:43 ` Alex Bennée
2024-03-12 6:03 ` Pierrick Bouvier
2024-03-12 7:37 ` Pierrick Bouvier
2024-02-29 5:53 ` [PATCH 5/5] tests/plugin/inline: add test for condition callback Pierrick Bouvier
2024-03-08 10:41 ` [PATCH 0/5] TCG plugins new inline operations Pierrick Bouvier
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).